-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Echo string format #76
Echo string format #76
Conversation
if len(marker_parts) == 2 | ||
let text = text . ' (' . marker_parts[1] . ')' | ||
endif | ||
let text = l:match[3] . ' (' . marker_parts[1] . ')' |
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 the length here is sometimes 2
, and sometimes 1
, because you sometimes get something like Warning/comma-dangle
and at other times just Error
. I'd probably leave this part as it is.
*ale* | ||
|
||
ALE - Asychronous Lint Engine | ||
ALE - Asynchronous Lint Engine |
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 totally missed that. Nice...
This variable defines the format of the echoed message. The `%s` is the | ||
error message itself, and it can contain the following handlers: | ||
- `%linter%` for linter's name | ||
- `%severity%` for the type of severity |
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.
Could you add a paragraph here with an example string for setting it up how you like it? I suspect others will want the same thing.
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.
Better to add it in the README.
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.
Maybe. One thing I found really helpful when trying out vim-airline was that I could type :help some_variable
, and the section would probably have some good suggestions on how to set the variables up. That was how I figured out how to get the triangular arrows to work.
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.
Let it be a part of the future refactoring, add examples in the doc.
" Return a formatted message according to g:ale_echo_msg_format variable | ||
function! s:GetMessage(linter, type, text) abort | ||
let msg = g:ale_echo_msg_format | ||
let linter = a:linter |
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 assignment is redundant.
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.
Are you talking about the comment?
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.
You can use a:linter
directly below.
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.
Ah ok, the linter variable is there for debugging purpose and as usually I forgot to remove it.
function! s:GetMessage(linter, type, text) abort | ||
let msg = g:ale_echo_msg_format | ||
let linter = a:linter | ||
let type = a:type ==# 'E' ? 'Error' : 'Warning' |
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 wonder if the severity strings should be configurable too. That might be handy for supporting other languages, or abbreviations, etc.
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 don't really see the need. It may be helpful if you want some eye candy on the message, using some fancy unicode characters.
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.
Japanese users may wish to display "でたらめ" instead of "Error" I suppose.
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.
If that means Error, so maybe 😄 I'll add a commit for that then.
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.
It's actually as near a translation of "bullshit" as I could find.
@@ -81,7 +95,9 @@ function! ale#cursor#EchoCursorWarning(...) | |||
let index = s:BinarySearch(loclist, pos[1], pos[2]) | |||
|
|||
if index >= 0 | |||
call ale#cursor#TruncatedEcho(loclist[index]['text']) | |||
let l = loclist[index] | |||
let msg = s:GetMessage(l.linter_name, l.type, l.text) |
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 wonder if it might be easier to pass the whole loclist item to the function instead.
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.
It is a good idea if we are planning to pass another variables in the future, but I cannot figure out what kind of variables.
Also even if its non noticeable, passing a big dictionnary may take some extra time for processing (a few milliseconds).
So as you wish, should I pass the whole loclist?
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.
It takes more time? It will just be passing a reference to the dictionary.
You can leave it how it is. It's just an observation.
Nice work on this. 👍 Check out my comments above. |
Via `g:ale_echo_msg_format` where: - `%s` is the error message itself - `%linter%` is the linter name - `%severity` is the severity type e.g let g:ale_echo_msg_fomat = '[%linter%] [%severity%] %s'
c82f52b
to
0fc4132
Compare
Check the last changes. The error and warning strings can now be defined. |
function! s:GetMessage(linter, type, text) abort | ||
let msg = g:ale_echo_msg_format | ||
let type = a:type ==# 'E' ? g:ale_echo_msg_error_str : | ||
\ g:ale_echo_msg_warning_str |
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 only have a nitpick here. I'd write this instead.
let type = a:type ==# 'E'
\ ? g:ale_echo_msg_error_str
\ : g:ale_echo_msg_warning_str
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'm more a ...
let type = a:type ==# 'E' ?
\ g:ale_echo_msg_error_str :
\ g:ale_echo_msg_warning_str
... person 😃
I added one nitpick above. Apart from that, it looks good to me. 👍 |
message `g:ale_echo_msg_error_str` and `g:ale_echo_msg_warning_str`
Now that the echoed message can be customized, no need to add the type to the text variable.
* Fix some typos * Sort the table of options alphabetically (except echo_msg_x_str options)
0fc4132
to
a56aae5
Compare
Looks like I overwrite your changes by force pushing, but it should be good now. |
Default: `Warning` | ||
|
||
The string used for warning severity in the echoed message. | ||
Note |`g:ale_echo_cursor`| should be setted to 1 |
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.
That should read "should be set". I'll merge this PR and change those phrases.
I just squash merged this. Nice stuff. Cheers! 🍻 |
The implementation is done, so for example in a simple python file the following:
will echo
'[flake8] [Error] E302: expected 2 blank lines, found 1'