From mboxrd@z Thu Jan 1 00:00:00 1970
From: Bernhard Bitsch <Bernhard.Bitsch@gmx.de>
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:
<trinity-ce9ab783-b451-40c5-872d-375341376f5b-1559743390273@3c-app-gmx-bs09>
In-Reply-To: <89A6CDA0-0D07-4A9C-B08F-EAA98DDD5B8D@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============6047810182203114138=="
List-Id: <development.lists.ipfire.org>
--===============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" <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
>
>=20
>=20
> > On 5 Jun 2019, at 14:10, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
> >=20
> > Hi,
> >=20
> >> 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
> >>=20
> >> Hi,
> >>=20
> >>> On 5 Jun 2019, at 12:13, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wro=
te:
> >>>=20
> >>> Hello,
> >>>=20
> >>> Thanks for the merge.
> >>>=20
> >>>> 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
> >>>>=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 <Bernhard.Bitsch(a)gmx.de> w=
rote:
> >>>>>=20
> >>>>> Save fixed leases to file after addition of a new lease
> >>>>>=20
> >>>>> Signed-off-by: Bernhard Bitsch <bbitsch(a)ipfire.org>
> >>>>>=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==--