-
Notifications
You must be signed in to change notification settings - Fork 626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autoscroll colorify compiler output #1139
Conversation
It is possible to scroll normally by page up/cursors/etc. – after leaving the bottom position of the view, it will be NOT auto-scrolled – only after again jumping to the last line the auto-scrolling will resume.
Does this PR have a chance of a merge, @koutcher ? I see no merges of PRs in this project. But because of quality of the code and its compactness, maybe you could merge? The patch can be seen/verified from inside out, that's small it is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sharing but this seems quite specific to your use case.
The autoscroll feature should not be active by default - Tig is emulating less
.
Autoscrolling makes a :!git log
deviate from less
.
* | ||
*/ | ||
regex_txt = "([^:]+):([0-9]+):(|([0-9]+):)[ \t]+(note|warning|error): (.*)(\\[-W([a-zA-Z0-8_-]+)\\]|)"; | ||
regex_err = regcomp(®ex, regex_txt, regex_flags); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'd need to call regfree
to avoid a memory leak
* {file}:{line}:{col}: (note|warning|error):{message} | ||
* | ||
*/ | ||
regex_txt = "([^:]+):([0-9]+):(|([0-9]+):)[ \t]+(note|warning|error): (.*)(\\[-W([a-zA-Z0-8_-]+)\\]|)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really specific. I think this should either be handled by a user-settable regex, or by an external program.
We could teach Tig to preserve ANSI escape codes in the output, then something like :!ls --color
would work as well.
Thanks for your contribution. I tend to agree with Johannes on the specificity of the use case, especially we shall not embed hardcoded patterns. I'll leave it to Jonas to decide. |
In a few days I'll provide an updated patch that will address the points, e.g.: will use an option for the regex. |
Hi,
I think that this PR implements quite a valuable features, which all converge to a very useful use case (explained at the end). The features are:
The patches
Autoscroll of the pager view – quite a missed feature, with a protection against overactivity – the view has to be placed at the bottom for the autoscroll to occur – i.e.: it's safe to scroll up and have a stable view of what's going on.
Colorizing of the compiler's errors and warnings (and notes) in pager view.
The errors are to be browsed in the use case explained below, so they are being colorized first here by this patch.
The use case
The two patches open a way for a configuring of TIG as a comfortable IDE-like errors round-trip fixing tool, by:
Configuring the
'n'
key in pager to search for lines with "error:|warning:|note:" regex, via.:Configuring Return in pager to open the editor on the line number extracted from the current error/warning/note, via:
Optionally, configuring
'E'
to run:!make
, via:Setting the
compiler-msg
color (the only thing that's problematic - I don't know how to make a default value for it), via:Then what's possible is the following use case:
.
Helper script body
The
make-line-open
script is as follows:Summary
The patches are quite simple, only 9 and 21 lines and follow the coding style. I think that the patches are useful also without/outside the use case – having pager view autoscrolled, for example, is an useful thing.