From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bernhard Bitsch To: development@lists.ipfire.org Subject: Aw: Re: [PATCH] dhcp.cgi: Fix for bug #12050 Date: Wed, 05 Jun 2019 16:03:10 +0200 Message-ID: In-Reply-To: <89A6CDA0-0D07-4A9C-B08F-EAA98DDD5B8D@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6047810182203114138==" List-Id: --===============6047810182203114138== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable > Gesendet: Mittwoch, 05. Juni 2019 um 15:26 Uhr > Von: "Michael Tremer" > An: "Bernhard Bitsch" > Cc: "IPFire Development" > Betreff: Re: [PATCH] dhcp.cgi: Fix for bug #12050 > >=20 >=20 > > 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 wro= te: > >>>=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= achieve here. It would have helped to add to the commit message that the exp= ected behaviour just wasn=E2=80=99t programmed into the file and that this pa= tch now changes that. > >>>>=20 > >>>=20 > >>> The commit message just describes what has changed. The new entry is ju= st 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 w= ay. > >>=20 > >> I tested the patch and I could add an extra from the fixed list with one= click. > >>=20 > >> What are you referring to? > >>=20 > >=20 > > Okay it is a mix-up. For editing the new entry is added to the fixed leas= es 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 t= o default state of the page. >=20 > This is intentional that the page remains in edit mode. People usually want= to change the remark or hostname. If you have a list of 200 leases, then fin= ding the one that you have just added can be a pain. That is why this is ther= e. > You can set these fields with the first addition operation. No need to make i= t easy editable in all cases. But the actual solution leaves a somehow irrita= ting state: data is displayed in the edit fields, top entry ( the entry just = added ) is in yellow; "is the entry added or must I click 'update' to accompl= ish this?" I'm talking of adding new fixed leases, not about adding dynamic leases to th= e fixed leases set. That's another case, where you should edit the data to se= parate the IP ranges for these sets. > >=20 > >>>=20 > >>>> I updated the commit message for your future reference. Please read th= rough that and take some inspiration from that with your next patch. > >>>>=20 > >>> I've read your message. If the patch contained this functionality I wou= ld 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 = file, without synching the sort order.=20 >=20 > Is that a problem when they are not sorted? By what would it be best to sor= t them? I do not think that the MAC address is a good option. Finding somethi= ng in there is impossible as soon as the list becomes large. File fixedleases is sorted by the setting 'SORT_FLEASES' from setttings. This= is true for the internal array in dhcp.cgi, also. >=20 > >=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 w= rote: > >>>>>=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{'= FIX_ROOTPATH'}); > >>>>> if ($dhcpsettings{'KEY2'} eq '') { #add or edit ? > >>>>> unshift (@current2, "$dhcpsettings{'FIX_MAC'},$dhcpsettings{'FIX= _ADDR'},$dhcpsettings{'FIX_ENABLED'},$dhcpsettings{'FIX_NEXTADDR'},$dhcpsetti= ngs{'FIX_FILENAME'},$dhcpsettings{'FIX_ROOTPATH'},$dhcpsettings{'FIX_REMARK'}= \n"); > >>>>> + open(FILE, ">$filename2") or die 'Unable to open fixed lease fi= le.'; > >>>>> + print FILE @current2; > >>>>> + close(FILE); > >>>>> &General::log($Lang::tr{'fixed ip lease added'}); > >>>>>=20 > >>>>> # Enter edit mode > >>>>> -- > >>>>> 2.21.0.windows.1 >=20 > --===============6047810182203114138==--