Signed-off-by: Stephen Cuka stephen@firemypi.org ---
This patch addresses an issue with the IPFire WebGUI adding a space after every 80 characters in the display of long log entries on the "Logs/System Logs" page. (Bug 13735)
The patch removes the "very basic breaking of lines..." code and replaces it with a direct copy of the log entry $data to the display output variable $d.
html/cgi-bin/logs.cgi/log.dat | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/html/cgi-bin/logs.cgi/log.dat b/html/cgi-bin/logs.cgi/log.dat index 01c382a0d..ce7de0178 100644 --- a/html/cgi-bin/logs.cgi/log.dat +++ b/html/cgi-bin/logs.cgi/log.dat @@ -412,11 +412,12 @@ foreach $_ (@log) $sec = 'kernel'; $data = $2.': '.$data; } - my $d = substr ($data,0,80); - while (length($data)>80){ #very basic breaking of lines... - $data = substr ($data,80); #permit correct display in table cell - $d .= ' ' . substr ($data,0,80); - } + #my $d = substr ($data,0,80); + #while (length($data)>80){ #very basic breaking of lines... + # $data = substr ($data,80); #permit correct display in table cell + # $d .= ' ' . substr ($data,0,80); + #} + my $d = $data; #don't break lines for display my $col="";
if ($lines % 2) {
Reviewed-by: Bernhard Bitsch bbitsch@ipfire.org Tested-by: Bernhard Bitsch bbitsch@ipfire.org
Am 04.08.2024 um 00:36 schrieb Stephen Cuka:
Signed-off-by: Stephen Cuka stephen@firemypi.org
This patch addresses an issue with the IPFire WebGUI adding a space after every 80 characters in the display of long log entries on the "Logs/System Logs" page. (Bug 13735)
The patch removes the "very basic breaking of lines..." code and replaces it with a direct copy of the log entry $data to the display output variable $d.
html/cgi-bin/logs.cgi/log.dat | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/html/cgi-bin/logs.cgi/log.dat b/html/cgi-bin/logs.cgi/log.dat index 01c382a0d..ce7de0178 100644 --- a/html/cgi-bin/logs.cgi/log.dat +++ b/html/cgi-bin/logs.cgi/log.dat @@ -412,11 +412,12 @@ foreach $_ (@log) $sec = 'kernel'; $data = $2.': '.$data; }
- my $d = substr ($data,0,80);
- while (length($data)>80){ #very basic breaking of lines...
$data = substr ($data,80); #permit correct display in table cell$d .= ' ' . substr ($data,0,80);- }
#my $d = substr ($data,0,80);
#while (length($data)>80){ #very basic breaking of lines...
# $data = substr ($data,80); #permit correct display in table cell
# $d .= ' ' . substr ($data,0,80);
#}
my $d = $data; #don't break lines for display my $col="";
if ($lines % 2) {
Reviewed-by: Adolf Belka adolf.belka@ipfire.org
Hi Stephen, Welcome to the list and thanks for the patch.
On 04/08/2024 00:36, Stephen Cuka wrote:
Signed-off-by: Stephen Cuka stephen@firemypi.org
This patch addresses an issue with the IPFire WebGUI adding a space after every 80 characters in the display of long log entries on the "Logs/System Logs" page. (Bug 13735)
The patch removes the "very basic breaking of lines..." code and replaces it with a direct copy of the log entry $data to the display output variable $d.
html/cgi-bin/logs.cgi/log.dat | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/html/cgi-bin/logs.cgi/log.dat b/html/cgi-bin/logs.cgi/log.dat index 01c382a0d..ce7de0178 100644 --- a/html/cgi-bin/logs.cgi/log.dat +++ b/html/cgi-bin/logs.cgi/log.dat @@ -412,11 +412,12 @@ foreach $_ (@log) $sec = 'kernel'; $data = $2.': '.$data; }
- my $d = substr ($data,0,80);
- while (length($data)>80){ #very basic breaking of lines...
$data = substr ($data,80); #permit correct display in table cell$d .= ' ' . substr ($data,0,80);- }
Removing these lines seems fine to me. The code looks to be adding a space instead of doing an actual line break. The text is also auto word wrapped in the text box in the browser anyway.
- #my $d = substr ($data,0,80);
- #while (length($data)>80){ #very basic breaking of lines...
- # $data = substr ($data,80); #permit correct display in table cell
- # $d .= ' ' . substr ($data,0,80);
- #}
My only question is why you left all the removed lines in place as comments, rather than just removing them completely?
- my $d = $data; #don't break lines for display
I think you could also save a line by not having this line but changing the earlier line of $data = $2.': '.$data;
to
my $d = $2.': '.$data;
Regards, Adolf
my $col="";
if ($lines % 2) {
Hi Adolf,
Thanks for your help on this.
I left the original code lines in place for context. They're not really necessary. If it's better, I can remove them and resubmit the patch.
For the
my $d = $2.': '.$data;
suggestion, there is some special processing for the display for the RED section logs.
# correct the cut position, just when section=RED if (($cgiparams{'SECTION'} eq 'red' ) && ($sec =~ /(kernel:)(.*)/)) { $sec = 'kernel'; $data = $2.': '.$data; }
Using 'my $d = $2. ': ' .$data;' for everything would add the timestamp to the front of the log text displayed for all sections.
I'm not sure if the special processing for $data for RED is necessary. I think that the point of the 'if' statement is to clean up the Section display for entries from 'kernel: ippp\d' and 'kernel: isdn.*' matches for RED, but I don't know why adding the timestamp to the front of data would be desirable in that specific case. I don't have any log entries to reference for that though.
Thanks, Stephen
Hi Stephen,
On 05/08/2024 20:47, Stephen Cuka wrote:
Hi Adolf,
Thanks for your help on this.
I left the original code lines in place for context. They're not really necessary. If it's better, I can remove them and resubmit the patch.
I think it would make more sense to remove them.
You will need to make it a v2 patch so it supersedes this one automatically in patchwork.
For the
my $d = $2.': '.$data;
suggestion, there is some special processing for the display for the RED section logs.
# correct the cut position, just when section=RED if (($cgiparams{'SECTION'} eq 'red' ) && ($sec =~ /(kernel:)(.*)/)) { $sec = 'kernel'; $data = $2.': '.$data; }Using 'my $d = $2. ': ' .$data;' for everything would add the timestamp to the front of the log text displayed for all sections.
I'm not sure if the special processing for $data for RED is necessary. I think that the point of the 'if' statement is to clean up the Section display for entries from 'kernel: ippp\d' and 'kernel: isdn.*' matches for RED, but I don't know why adding the timestamp to the front of data would be desirable in that specific case. I don't have any log entries to reference for that though.
I hadn't seen the special processing for the RED section logs.
I would leave it as you have done it then unless Michael or Arne or Peter come back further on this.
Let's keep it simple for now.
Regards,
Adolf.
Thanks, Stephen
Hello Stephen,
Welcome to the list!
On 5 Aug 2024, at 19:47, Stephen Cuka stephen@firemypi.org wrote:
Hi Adolf,
Thanks for your help on this.
I left the original code lines in place for context. They're not really necessary. If it's better, I can remove them and resubmit the patch.
For the
my $d = $2.': '.$data;
suggestion, there is some special processing for the display for the RED section logs.
# correct the cut position, just when section=RED if (($cgiparams{'SECTION'} eq 'red' ) && ($sec =~ /(kernel:)(.*)/)) { $sec = 'kernel'; $data = $2.': '.$data; }Using 'my $d = $2. ': ' .$data;' for everything would add the timestamp to the front of the log text displayed for all sections.
I'm not sure if the special processing for $data for RED is necessary. I think that the point of the 'if' statement is to clean up the Section display for entries from 'kernel: ippp\d' and 'kernel: isdn.*' matches for RED, but I don't know why adding the timestamp to the front of data would be desirable in that specific case. I don't have any log entries to reference for that though.
I think we can safely drop this as we no longer support ISDN.
-Michael
Thanks, Stephen
-
Adolf Belka -
Bernhard Bitsch -
Michael Tremer -
Stephen Cuka