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

Add option to control the message length in flycheck-error-list-mode #828

Closed
vmalloc opened this Issue Dec 23, 2015 · 17 comments

Comments

Projects
None yet
3 participants
@vmalloc

vmalloc commented Dec 23, 2015

Currently messages are automatically truncated and often times make it hard to understand the issue. It seems that wider windows don't extend this truncation, so it would be very nice to have a configuration/customization option to change this behavior

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 23, 2015

The error list does not constrain the length of error messages. The column has no maximum width, as such the message should never be truncated as long as there's still space left in the window.

Can you show a screenshot of an undesired or faulty truncation?

@vmalloc

This comment has been minimized.

vmalloc commented Dec 23, 2015

Here is a basic use case with pylint: https://i.imgur.com/E1LfKkr.png

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 24, 2015

@vmalloc I'm sorry, but I do not see what the issue is in this screenshot. To me the message does not appear to be truncated; only the error ID is. Would you mind to annotate the screenshot to illustrate where exactly the problem appears?

@vmalloc

This comment has been minimized.

vmalloc commented Dec 24, 2015

First, the ID is indeed truncated as well, but I'm not sure about the ellipsis in the error message. If it's not truncated - why the ellipsis?

@vmalloc

This comment has been minimized.

vmalloc commented Dec 24, 2015

The full error for the first one should be Final newline missing [missing-final-newline] for instance

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 24, 2015

Why do you think so? The text between the brackets is the error ID which we parse separately and not as part of the error message.

The ellipsis comes from Tabulated List Mode; it does not indicate truncation.

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 24, 2015

Would that be better?

bildschirmfoto 2015-12-24 um 16 45 01

lunaryorn added a commit that referenced this issue Dec 24, 2015

Truncate error list with spaces
Don’t use ellipsis to not suggest truncation where none occurs.

See GH-828

@lunaryorn lunaryorn self-assigned this Dec 24, 2015

@vmalloc

This comment has been minimized.

vmalloc commented Dec 24, 2015

Yes! How do I configure it to appear that way?
On יום ה׳, 24 בדצמ׳ 2015 at 17:45 Sebastian Wiesner <
notifications@github.com> wrote:

Would that be better?

[image: bildschirmfoto 2015-12-24 um 16 45 01]
https://cloud.githubusercontent.com/assets/224922/11996148/b8bd5bda-aa5d-11e5-9509-73bfcd831e82.png


Reply to this email directly or view it on GitHub
#828 (comment).

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 24, 2015

@vmalloc You can't unfortunately. We change Flycheck to appear that way, but this change will only work on the not yet released Emacs 25 because the necessary customisation option is only available in this version.

Implementing a similar change on earlier Emacs versions would amount to a re-write of parts of Tabulated List Mode, which is more effort than it is worth, at least as far as I'm concerned.

@cpitclaudel If you agree I'd push this change.

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Dec 24, 2015

Looks good to me! (It's not obvious to me why tabulated-list-mode uses truncate-string-ellipsis here at all, though)

cpitclaudel added a commit that referenced this issue Dec 25, 2015

Merge the last two columns of the error list.
This allows the message column to stretch arbitrarily far, without
adding meaningless ellipsis.

Fixes GH-828.
@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Dec 25, 2015

Btw, there may be a way to support this in Emacs 24.3 without duplicating code. IIUC the problem comes from the fact that we want the second to last column to be of unbounded width (instead of the last one, which has the checker). We thus declare it to be of width 0, which tricks truncate-string (called by tabluated-mode) into not actually truncating our string ­– but it still inserts these pesky "...". And IIUC, in Emacs 25 we can actually tell the string truncation function to use "" instead of "...", making the "truncation" a full no-op.

Here are potential solutions that would work in 24:

  • Move the name of the checker to before the message (in the list).
  • Make the list of columns and their widths customizable, so that users can reorder columns (some work involved in parametrizing the function that constructs entries)
  • Merge the last two columns into a single one.

That last one is actually my favourite; I've implemented is in branch fix-828. The only technical issue with it is fontification: we still want the checker name to look different. I did this by manually doing the fontification for the last column instead of passing a face to insert-text-button.

Note that this has one weakness: it doesn't let you sort by checker — that could well be a deal breaker. There might also be plugins out there that depend on the exact columns that flycheck's error list uses.

@vmalloc

This comment has been minimized.

vmalloc commented Dec 25, 2015

Ok, I can wait until Emacs 25 ships for this. Thanks!

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 25, 2015

@cpitclaudel I like the idea of combining both columns; in fact Flycheck used to do that initially, but moved to a separate column for the checker in #500. The reason was theming of the checker name, but if we can preserve that feature with a single column nothing stands against a single column, imho.

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Jan 10, 2016

@vmalloc The error message and the syntax checker are now in a single column, removing the redundant ellipsis.

@cpitclaudel I merged your change, and updated the test cases accordingly. I took the opportunity to play with Buttercup; I found that it's really much much better than ERT for this kind of tests. I think I'll slowly port most—if not all—of our tests over to Buttercup specs, i.e. whenever I touch tests for a specific piece of Flycheck, I'll move them to buttercup first. It's really a lot easier to read and write 😍

lunaryorn added a commit that referenced this issue Jan 10, 2016

Restore Emacs 24.3 compatibility
Don’t use add-face-text-property, which was only introduced in 24.4

See GH-828
@vmalloc

This comment has been minimized.

vmalloc commented Jan 10, 2016

@lunaryorn Awesome! Thanks!

lunaryorn added a commit that referenced this issue Jan 10, 2016

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Jan 10, 2016

@lunaryorn Thanks for polishing up the patch and merging it :) Indeed, the new tests look beautiful!

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Jan 10, 2016

@cpitclaudel You're welcome. Thanks for this nice solution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment