Aw: Re: [PATCH] dhcp.cgi: Fix for bug #12050

[Bernhard Bitsch <Bernhard.Bitsch@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 3545 bytes --]

Hi,

> Gesendet: Mittwoch, 05. Juni 2019 um 14:06 Uhr
> Von: "Michael Tremer" <michael.tremer(a)ipfire.org>
> An: "Bernhard Bitsch" <Bernhard.Bitsch(a)gmx.de>
> Cc: "IPFire Development" <development(a)lists.ipfire.org>
> Betreff: Re: [PATCH] dhcp.cgi: Fix for bug #12050
>
> Hi,
> 
> > On 5 Jun 2019, at 12:13, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
> > 
> > Hello,
> > 
> > Thanks for the merge.
> > 
> >> Gesendet: Mittwoch, 05. Juni 2019 um 11:05 Uhr
> >> Von: "Michael Tremer" <michael.tremer(a)ipfire.org>
> >> An: "Bernhard Bitsch" <Bernhard.Bitsch(a)gmx.de>
> >> Cc: "IPFire Development" <development(a)lists.ipfire.org>
> >> Betreff: Re: [PATCH] dhcp.cgi: Fix for bug #12050
> >> 
> >> Hello,
> >> 
> >> I merged this patch.
> >> 
> >> 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 expected behaviour just wasn’t programmed into the file and that this patch now changes that.
> >> 
> > 
> > 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.
> 
> I tested the patch and I could add an extra from the fixed list with one click.
> 
> What are you referring to?
> 

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.

> > 
> >> I updated the commit message for your future reference. Please read through that and take some inspiration from that with your next patch.
> >> 
> > I've read your message. If the patch contained this functionality I would have stated that in this way.
> 
> What does the patch do from your point of view?
> 

It only synchronises the internal fixed leases list and the fixed leases file, without synching the sort order. 

> > 
> >> I am happy that we can finally close this bug.
> >> 
> > So am I.
> > 
> > Best,
> > Bernhard
> > 
> >> Best,
> >> -Michael
> >> 
> >>> On 4 Jun 2019, at 11:24, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
> >>> 
> >>> Save fixed leases to file after addition of a new lease
> >>> 
> >>> Signed-off-by: Bernhard Bitsch <bbitsch(a)ipfire.org>
> >>> 
> >>> ---
> >>> html/cgi-bin/dhcp.cgi | 3 +++
> >>> 1 file changed, 3 insertions(+)
> >>> 
> >>> 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'} = &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'},$dhcpsettings{'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'});
> >>> 
> >>> 	    # Enter edit mode
> >>> --
> >>> 2.21.0.windows.1
> >>> 
> >> 
> >> 
> 
>