From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] improve DHCP dynamic leases list usability Date: Mon, 19 Oct 2020 11:26:37 +0100 Message-ID: <8DD762BC-3ED0-47E9-9C90-621B343D2B8F@ipfire.org> In-Reply-To: <3e1fa0ad-9391-362b-9b3e-550333dd565b@leo-andres.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3337692219620557314==" List-Id: --===============3337692219620557314== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Good Morning Leo, Thank you very much for submitting your patch. I agree that this is an issue that could be fixed. However, your patch does n= ot apply. It seems to have been corrupted while being sent. root(a)michael:/build/ipfire-2.x# pwclient git-am -s 3586 Applying patch #3586 using u'git am -s' Description: improve DHCP dynamic leases list usability warning: Patch sent with format=3Dflowed; space at the end of lines might be = lost. Applying: improve DHCP dynamic leases list usability error: corrupt patch at line 10 Patch failed at 0001 improve DHCP dynamic leases list usability hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". 'git am' failed with exit status 128 You seem to have re-indented the code as well as making changes to it which m= akes reviewing very difficult. It would be better to do this in two steps: a)= fix the code that is messy, and then b) extend it with new functionality. Could you please try to rebase this patch against the next branch and submit = again? Please let me know if you need any help with this. Best, -Michael > On 18 Oct 2020, at 23:19, Leo Hofmann wrote: >=20 > Some users have complained that displaying the active and expired leases in= one > unordered list is confusing. This patch improves the DHCP user interface by > visually separating active and expired leases. The list is now sorted by ac= tive and > expired leases and they are divided by a horizontal line. > Sorting by IP/MAC/host/time and creating static leases remains unchanged. >=20 > See forum topic for context: https://community.ipfire.org/t/dhcp-server-exp= ired-leases/ >=20 > Signed-off-by: Leo-Andres Hofmann >=20 > --- > config/cfgroot/header.pl | 167 +++++++++++++++++++++++---------------- > 1 file changed, 97 insertions(+), 70 deletions(-) >=20 > diff --git a/config/cfgroot/header.pl b/config/cfgroot/header.pl > index 1046f5992..b5d153a55 100644 > --- a/config/cfgroot/header.pl > +++ b/config/cfgroot/header.pl > @@ -403,88 +403,110 @@ sub PrintActualLeases > <= b>$tr{'mac address'} > $tr{'hostname'} > $tr{'lease expires'} (local time d/m/y) > -Add to fix leases > +Add to fix leases > > END > ; >=20 > open(LEASES,"/var/state/dhcp/dhcpd.leases") or die "Can't open dhcpd.l= eases"; > - while ($line =3D ) { > - next if( $line =3D~ /^\s*#/ ); > - chomp($line); > - @temp =3D split (' ', $line); > - > - if ($line =3D~ /^\s*lease/) { > - $ip =3D $temp[1]; > - #All field are not necessarily read. Clear everything > - $endtime =3D 0; > - $ether =3D ""; > - $hostname =3D ""; > - } > + while (my $line =3D ) { > + next if( $line =3D~ /^\s*#/ ); > + chomp($line); > + @temp =3D split (' ', $line); > + > + if ($line =3D~ /^\s*lease/) { > + $ip =3D $temp[1]; > + #All field are not necessarily read. Clear everything > + $endtime =3D 0; > + $endtime_print =3D ""; > + $expired =3D 0; > + $ether =3D ""; > + $hostname =3D ""; > + } >=20 > - if ($line =3D~ /^\s*ends/) { > - $line =3D~ /(\d+)\/(\d+)\/(\d+) (\d+):(\d+):(\d+)/; > - $endtime =3D timegm($6, $5, $4, $3, $2 - 1, $1 - 1900); > - } > + if ($line =3D~ /^\s*ends/) { > + $line =3D~ /(\d+)\/(\d+)\/(\d+) (\d+):(\d+):(\d+)/; > + $endtime =3D timegm($6, $5, $4, $3, $2 - 1, $1 - 1900); > + ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $dst) = =3D localtime($endtime); > + $endtime_print =3D sprintf ("%02d/%02d/%d %02d:%02d:%02d",$mda= y,$mon+1,$year+1900,$hour,$min,$sec); > + $expired =3D $endtime < time(); > + } >=20 > - if ($line =3D~ /^\s*hardware ethernet/) { > - $ether =3D $temp[2]; > - $ether =3D~ s/;//g; > - } > + if ($line =3D~ /^\s*hardware ethernet/) { > + $ether =3D $temp[2]; > + $ether =3D~ s/;//g; > + } >=20 > - if ($line =3D~ /^\s*client-hostname/) { > - $hostname =3D "$temp[1] $temp[2] $temp[3]"; > - $hostname =3D~ s/;//g; > - $hostname =3D~ s/\"//g; > - } > + if ($line =3D~ /^\s*client-hostname/) { > + $hostname =3D "$temp[1] $temp[2] $temp[3]"; > + $hostname =3D~ s/;//g; > + $hostname =3D~ s/\"//g; > + } >=20 > - if ($line eq "}") { > - @record =3D ('IPADDR',$ip,'ENDTIME',$endtime,'ETHER',$ether,'HOSTN= AME',$hostname); > - $record =3D {}; # create a refe= rence to empty hash > - %{$record} =3D @record; # populate that has= h with @record > - $entries{$record->{'IPADDR'}} =3D $record; # add this to a h= ash of hashes > - } > + if ($line eq "}") { > + @record =3D ('IPADDR',$ip,'ENDTIME',$endtime,'ETHER',$ether,'H= OSTNAME',$hostname,'endtime_print',$endtime_print,'expired',$expired); > + $record =3D {}; # create a refe= rence to empty hash > + %{$record} =3D @record; # populate that= hash with @record > + $entries{$record->{'IPADDR'}} =3D $record; # add this to a = hash of hashes > + } > } > close(LEASES); >=20 > my $id =3D 0; > - my $col=3D""; > + my $col =3D ""; > + my $separator_printed =3D 0; > foreach my $key (sort leasesort keys %entries) { > - print "
\n"; > - my $hostname =3D &cleanhtml($entries{$key}->{HOSTNAME},"y"); > - > - if ($id % 2) { > - print ""; > - $col=3D"bgcolor=3D'$table1colour'"; > - } > - else { > - print ""; > - $col=3D"bgcolor=3D'$table2colour'"; > - } > - > - print < + my $hostname =3D &cleanhtml($entries{$key}->{HOSTNAME},"y"); > + my $hostname_print =3D $hostname; > + if($hostname_print eq "") { #print blank space if no hostname is f= ound > + $hostname_print =3D "   "; > + } > + > + # separate active and expired leases with a horizontal line > + if(($entries{$key}->{expired}) && ($separator_printed =3D=3D 0)) { > + $separator_printed =3D 1; > + if ($id % 2) { > + print "\n"; > + } else { > + print "\n"; > + } > + $id++; > + } > + > + print "\n"; > + if ($id % 2) { > + print ""; > + $col=3D"bgcolor=3D'$table1colour'"; > + } else { > + print ""; > + $col=3D"bgcolor=3D'$table2colour'"; > + } > + > + if($entries{$key}->{expired}) { > + print < +{IPADDR}' />$entries{$key}->{IPADDR} > +{ETHER}' />$entries{$key}->{ETHER} > +$hostname_print > +$entries{$key}->{endtime_print} > +END > +; > + } else { > + print < {IPADDR}' />$entries{$key}->{IPADDR} > {ETHER}' />$entries{$key}->{ETHER} > - $hostname > - > +$hostname_print > +$entries{$key}->{endtime_print} > END > -; > - > - ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $dst) =3D localt= ime ($entries{$key}->{ENDTIME}); > - $enddate =3D sprintf ("%02d/%02d/%d %02d:%02d:%02d",$mday,$mon+1,$year= +1900,$hour,$min,$sec); > - > - if ($entries{$key}->{ENDTIME} < time() ){ > - print "$enddate"; > - } else { > - print "$enddate"; > - } > - print < - > - > +; > + } > + > + print < + > + > END > -; > - $id++; > - } > +; > + $id++; > + } >=20 > print ""; > &closebox(); > @@ -499,11 +521,14 @@ sub leasesort { > if ($qs eq 'IPADDR') { > @a =3D split(/\./,$entries{$a}->{$qs}); > @b =3D split(/\./,$entries{$b}->{$qs}); > + $entries{$a}->{'expired'} <=3D> $entries{$b}->{'expired'} || > ($b[0]<=3D>$a[0]) || > ($b[1]<=3D>$a[1]) || > ($b[2]<=3D>$a[2]) || > ($b[3]<=3D>$a[3]); > - }else { > + ; > + } else { > + $entries{$a}->{'expired'} <=3D> $entries{$b}->{'expired'} || > $entries{$b}->{$qs} cmp $entries{$a}->{$qs}; > } > } > @@ -511,15 +536,17 @@ sub leasesort { > { > $qs=3D$dhcpsettings{'SORT_LEASELIST'}; > if ($qs eq 'IPADDR') { > - @a =3D split(/\./,$entries{$a}->{$qs}); > + @a =3D split(/\./,$entries{$a}->{$qs}); > @b =3D split(/\./,$entries{$b}->{$qs}); > + $entries{$a}->{'expired'} <=3D> $entries{$b}->{'expired'} || > ($a[0]<=3D>$b[0]) || > - ($a[1]<=3D>$b[1]) || > - ($a[2]<=3D>$b[2]) || > + ($a[1]<=3D>$b[1]) || > + ($a[2]<=3D>$b[2]) || > ($a[3]<=3D>$b[3]); > - }else { > + } else { > + $entries{$a}->{'expired'} <=3D> $entries{$b}->{'expired'} || > $entries{$a}->{$qs} cmp $entries{$b}->{$qs}; > - } > + } > } > } >=20 >=20 >=20 --===============3337692219620557314==--