From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] dhcp.cgi: Fix for bug #12050 Date: Wed, 05 Jun 2019 14:26:44 +0100 Message-ID: <89A6CDA0-0D07-4A9C-B08F-EAA98DDD5B8D@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2057126014524088527==" List-Id: --===============2057126014524088527== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable > On 5 Jun 2019, at 14:10, Bernhard Bitsch wrote: >=20 > Hi, >=20 >> Gesendet: Mittwoch, 05. Juni 2019 um 14:06 Uhr >> Von: "Michael Tremer" >> An: "Bernhard Bitsch" >> Cc: "IPFire Development" >> Betreff: Re: [PATCH] dhcp.cgi: Fix for bug #12050 >>=20 >> Hi, >>=20 >>> On 5 Jun 2019, at 12:13, Bernhard Bitsch wrote: >>>=20 >>> Hello, >>>=20 >>> Thanks for the merge. >>>=20 >>>> Gesendet: Mittwoch, 05. Juni 2019 um 11:05 Uhr >>>> Von: "Michael Tremer" >>>> An: "Bernhard Bitsch" >>>> Cc: "IPFire Development" >>>> Betreff: Re: [PATCH] dhcp.cgi: Fix for bug #12050 >>>>=20 >>>> Hello, >>>>=20 >>>> I merged this patch. >>>>=20 >>>> I had to spend a little time to figure out what you actually wanted to a= chieve here. It would have helped to add to the commit message that the expec= ted behaviour just wasn=E2=80=99t programmed into the file and that this patc= h now changes that. >>>>=20 >>>=20 >>> The commit message just describes what has changed. The new entry is just= added to the file. >>> It is not the "one-click-solution" I would prefer, too. >>> I can submit a patch for this functionality, if we want to do it this way. >>=20 >> I tested the patch and I could add an extra from the fixed list with one c= lick. >>=20 >> What are you referring to? >>=20 >=20 > Okay it is a mix-up. For editing the new entry is added to the fixed leases= list. Thus if I skip the edit step, the entry is contained anyway, but it is= not sorted in. > A one-click-solution should add the new entry, sort the list and return to = default state of the page. This is intentional that the page remains in edit mode. People usually want t= o change the remark or hostname. If you have a list of 200 leases, then findi= ng the one that you have just added can be a pain. That is why this is there. >=20 >>>=20 >>>> I updated the commit message for your future reference. Please read thro= ugh that and take some inspiration from that with your next patch. >>>>=20 >>> I've read your message. If the patch contained this functionality I would= have stated that in this way. >>=20 >> What does the patch do from your point of view? >>=20 >=20 > It only synchronises the internal fixed leases list and the fixed leases fi= le, without synching the sort order.=20 Is that a problem when they are not sorted? By what would it be best to sort = them? I do not think that the MAC address is a good option. Finding something= in there is impossible as soon as the list becomes large. >=20 >>>=20 >>>> I am happy that we can finally close this bug. >>>>=20 >>> So am I. >>>=20 >>> Best, >>> Bernhard >>>=20 >>>> Best, >>>> -Michael >>>>=20 >>>>> On 4 Jun 2019, at 11:24, Bernhard Bitsch wro= te: >>>>>=20 >>>>> Save fixed leases to file after addition of a new lease >>>>>=20 >>>>> Signed-off-by: Bernhard Bitsch >>>>>=20 >>>>> --- >>>>> html/cgi-bin/dhcp.cgi | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>>=20 >>>>> diff --git a/html/cgi-bin/dhcp.cgi b/html/cgi-bin/dhcp.cgi >>>>> index 675d80012..19c55eb6d 100644 >>>>> --- a/html/cgi-bin/dhcp.cgi >>>>> +++ b/html/cgi-bin/dhcp.cgi >>>>> @@ -443,6 +443,9 @@ if ($dhcpsettings{'ACTION'} eq $Lang::tr{'add'}.'2'= ) { >>>>> $dhcpsettings{'FIX_ROOTPATH'} =3D &Header::cleanhtml($dhcpsettings{'FI= X_ROOTPATH'}); >>>>> if ($dhcpsettings{'KEY2'} eq '') { #add or edit ? >>>>> unshift (@current2, "$dhcpsettings{'FIX_MAC'},$dhcpsettings{'FIX_A= DDR'},$dhcpsettings{'FIX_ENABLED'},$dhcpsettings{'FIX_NEXTADDR'},$dhcpsetting= s{'FIX_FILENAME'},$dhcpsettings{'FIX_ROOTPATH'},$dhcpsettings{'FIX_REMARK'}\n= "); >>>>> + open(FILE, ">$filename2") or die 'Unable to open fixed lease file= .'; >>>>> + print FILE @current2; >>>>> + close(FILE); >>>>> &General::log($Lang::tr{'fixed ip lease added'}); >>>>>=20 >>>>> # Enter edit mode >>>>> -- >>>>> 2.21.0.windows.1 --===============2057126014524088527==--