Wrap lines in log messages with line feeds #2967
Conversation
Signed-off-by: Chris Jackson <[email protected]>
#2630 Bundle Size — 10.97MiB (~+0.01%).b451e1a(current) vs 095584a main#2628(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #2630 |
Baseline #2628 |
|
|---|---|---|
 Initial JS |
1.9MiB |
1.9MiB |
| Initial CSS |
577.21KiB |
577.21KiB |
 Cache Invalidation |
17.4% |
22.71% |
| Chunks |
227 |
227 |
| Assets |
250 |
250 |
| Modules |
2949 |
2949 |
| Duplicate Modules |
154 |
154 |
| Duplicate Code |
1.78% |
1.78% |
| Packages |
98 |
98 |
| Duplicate Packages |
2 |
2 |
Bundle size by type 2 changes 
2 regressions
| Current #2630 |
Baseline #2628 |
|
|---|---|---|
| JS |
9.18MiB (~+0.01%) |
9.18MiB |
| CSS |
866.31KiB (~+0.01%) |
866.3KiB |
| Fonts |
526.1KiB |
526.1KiB |
| Media |
295.6KiB |
295.6KiB |
| IMG |
140.74KiB |
140.74KiB |
| HTML |
1.38KiB |
1.38KiB |
| Other |
871B |
871B |
Bundle analysis report Branch cdjackson:logviewer-supportlinef... Project dashboard
Generated by RelativeCI Documentation Report issue
|
Hello @openhab/webui-maintainers. Can you make the GHA CI build required instead of the Jenkins CI build? I disabled the PR builds in Jenkins for repos that already have a GHA CI build as a first step to migrate everything to GHA. |
| @@ -486,7 +485,7 @@ export default { | |||
| milliseconds: ms, | |||
| level: logEntry.level.toUpperCase(), | |||
| loggerName: logEntry.loggerName, | |||
| message: logEntry.message | |||
| message: logEntry.message.replace(/\n/g, '<br>') | |||
There was a problem hiding this comment.
I would add some indentation here as well, for example 2 spaces.
There was a problem hiding this comment.
I've not looked at a "real" stack trace - doesn't it have tabs (or whitespace of some description) after the break. In theory it should render with some indentation.
While I understand your thinking, I'm a little hesitant to add "random" indentation. In principal, this isn't stack trace specific - it should (IMHO) faithfully represent whatever was received in the logger message - so if the message has indentation, then we should also have some, but if not, we shouldn't add any extra (IMHO).
WDYT?
There was a problem hiding this comment.
I've not looked at a "real" stack trace - doesn't it have tabs (or whitespace of some description) after the break. In theory it should render with some indentation.
I was only looking at your screenshot above and wondered where the indentation was.
I think normally the exception message itself has intendation.
Will check this.
|
@wborn Done. We have to check if the requirement still works when upgrading the action and its name changes. |
|
I think this will break the "virtual table" from #2905 where every row is expected to be exactly 31 pixels high. Maybe another way to deal with these multi-line messages is to only display the first line and let the user know there's more. They can then click on the row (or an icon) to reveal the entire message in a popup. |
|
Interestingly I don't see any issues with this - what do you expect to happen @ghys ? Is it during the removal that lines might get out of sync or something? |
|
Looking at the code, it seems that the index of the rows might be wrong - but does that really matter? I mean, the user will scroll to the lines they want to see, so this is kind of self compensating - right? |
|
So what I can notice is when a stack trace comes in, the table only scrolls up 1 line rather than 4 (or whatever). This seems to trigger the detection that we're not at the bottom of the list, and the user needs to scroll down. After this, it works fine. Personally, I think this is ok. I don't think there should be loads of multi-line messages - although if there were it might get a bit messy. I'm also ok with @ghys suggestion to only show the first line and then have a popup if that's preferred. |
What I feared is that the scrolling would act up - I didn't test it - but glad to hear it's not so bad. It still would perhaps have inconsistent scrollbar behaviour because with a fixed row height, a table with x rows is a predictable 31 * x pixels high and doesn't depend on the height of individual entries, so you can easily calculate the height of the padding empty spaces on either side to maintain that height and the illusion of scrolling through a table with all entries rendered. |
This wraps log messages where the message has a line feed sequence. The main case for this is exception / stack traces.
Closes #2961