Improve Log Message Reporting

thomasjo / atom-latex

Compile LaTeX or knitr documents from within Atom

https://atom.io/packages/latex

MIT License

223 stars 42 forks source link

Improve Log Message Reporting #372

Closed yitzchak closed 7 years ago

yitzchak commented 7 years ago

This a PR to improve the log reporting UI. Various ideas are listed below.

[x] Use the new Docks API
[x] Use events to make message display real-time (not cached until end of build)
[x] Add message count indicator
[x] Update busy indicator and fix the horrible wobble of the icon
[x] Refresh log indicators on loggingLevel setting change
[x] Use table for message display
[x] Mark error locations in log files
[x] ~~Allow logging level to be set from the dock~~ Move to filtering UI
[x] ~~Add message copy~~ Move to separate PR
[x] ~~Add message filtering UI (#359)~~ Move to separate PR
[x] ~~Retain messages from previous builds~~ requires changes to log message format
[x] Serialize messages

yitzchak commented 7 years ago

@thomasjo Check out the attached screen capture showing usage of the Docks API, message counter indicator and updating the log in response to changes in loggingLevel.

screen capture

yitzchak commented 7 years ago

:fire: wobble in busy indicator.

peek 2017-05-19 08-09

yitzchak commented 7 years ago

Table based display of messages.

screenshot from 2017-05-21 11-37-31

yitzchak commented 7 years ago

Screenshot showing loggingLevel controls in dock.

peek 2017-05-23 09-25

thomasjo commented 7 years ago

Screenshot showing loggingLevel controls in dock.

I like the intent, but from a user experience perspective it's not clear what the buttons do, nor what actually happens when you click them. Maybe a tooltip is enough, but this likely needs a label or something near the buttons that conveys "Adjust logging level". Couple that with a descriptive tooltip that clearly states what logging level here means.

What do you think? :thought_balloon:

yitzchak commented 7 years ago

Agree on log filter UI. Will probably need multiple iterations so I am moving into separate PR.

yitzchak commented 7 years ago

@thomasjo I've removed the logging level buttons and done some code cleanup. I think this one should ready to go.

yitzchak commented 7 years ago

The test coverage went down by one because getMostSevereType is no longer being used (or DefaultLogger). I used CSS specificity instead to make the error markers in the gutter override the warning markers, etc. The advantage of this method is that if two message ranges overlap then the non-overlapping regions will still be styled correctly.

Aside from that, I added a few more tests and fixed the issues you noted. Once I get a green light from CI I'll probably pull the trigger! :tada:

yitzchak commented 7 years ago

@thomasjo I've managed to recover most of the tests that are still applicable. I'll take a look at specs for MarkerManager tomorrow. I think that will wrap it up.

yitzchak commented 7 years ago

Using Atom's loading indicator instead?

peek 2017-06-01 09-24