frescobaldi / frescobaldi

Frescobaldi LilyPond Editor
http://www.frescobaldi.org/
GNU General Public License v2.0
743 stars 153 forks source link

Error marks shouldn’t persist in included files #1441

Open sincere-music opened 2 years ago

sincere-music commented 2 years ago

Suppose I have two files:

% a.ly
\version "2.23.9"
\language "deutsch"
\include "b.ily"
% b.ily
{ h }

When I accidentally compile b.ily, Lily will throw an error for not a note name and Frescobaldi will add error marks and change the tab icon. So far, so good. However, when I then correctly compile a.ly, the error marks and tab icon for b.ily persist, although no longer relevant.

mr-bronson commented 1 year ago

You can clear the error marks manually by going to View --> Clear Error Marks. This simply removes the metadata that has been associated with that filename, specifically, a list of line numbers it thinks are in error. Thanks for pointing out that the error icon on the tab doesn't go away. Clearing error marks should make that go away but currently doesn't.

I do think the Frescobaldi "bookmarks" system needs a significant facelift, though, and I'm happy to re-write it when I have time, if no one minds. I would make errors track a particular character (the one the error message pointed to) rather than the line it pointed to. As it is, it's far too easy for the red lines to track irrelevant or blank lines once you start editing again, and they can even get expanded into large blocks of extra lines that were never a problem.

Before doing something like that, though, I would first want to get some feedback from users, like:

Is it ever helpful to you to have error marks stored between sessions? Should they only exist while the main document remains open?

Would you be okay with a highlighting mechanism that draws more attention to the particular character(s) that LilyPond is pointing out on a line, while still highlighting enough context to make errors very easy to locate visually?

Should warnings be highlighted differently from errors? (e.g. a different color)

Under what conditions would you prefer that the error marks disappear? Would you want a way to clear a current error mark without clearing all of them? In other words, you could unmark the errors as you fix them, so that once you deal with each of them, then you know you can try engraving again.

Would it be helpful to have the LilyPond Log widget track with where you are currently editing? That would avoid having to manually scroll through the error list. Just go to the next error mark (manually or by shortcut) and the error line associated with that will be visible in the Log widget. That seems relatively straightforward to implement, since there's already a relationship between each error line in the log and a position in the text.

The only other consumer of the bookmarks system is the Ctrl+B that marks a line, like inserting a break point in some other editors, but without any functionality. How do you use this feature? Or don't you? And would it be more helpful to mark specific selections (a character or multiple lines) instead of only marking by lines?

sincere-music commented 1 year ago

Thanks a lot for the consideration and offering to improve the code!

I can't reply in detail right now, but it seems to me that most of the questions you address would be solved by one fundamental change: error marks should remain tied to the LilyPond log file that triggered them. They should never persist once the “main” or “master” file associated with it has been closed or recompiled (they might be re-triggered, but even then they don't actually persist). Nor should they ideally show up in an \included file when in the meantime another “master” file that points to the same included file, but produces its own log, has been opened, let alone compiled.

I can't speak to implementation of any of that when multiple tabs are open.

I have used the Ctrl+B, not often, but I'm done situations it's really important. I'd be happy with it highlighting selections instead of lines.

Am 21.02.2023 - 20:52 schrieb mr-bronson:

You can clear the error marks manually by going to View --> Clear Error Marks. This simply removes the metadata that has been associated with that filename, specifically, a list of line numbers it thinks are in error. Thanks for pointing out that the error icon on the tab doesn't go away. Clearing error marks should make that go away but currently doesn't.

I do think the Frescobaldi "bookmarks" system needs a significant facelift, though, and I'm happy to re-write it when I have time, if no one minds. I would make errors track a particular character (the one the error message pointed to) rather than the line it pointed to. As it is, it's far too easy for the red lines to track irrelevant or blank lines once you start editing again, and they can even get expanded into large blocks of extra lines that were never a problem.

Before doing something like that, though, I would first want to get some feedback from users, like:

Is it ever helpful to you to have error marks stored between sessions? Should they only exist while the main document remains open?

Would you be okay with a highlighting mechanism that draws more attention to the particular character(s) that LilyPond is pointing out on a line, while still highlighting enough context to make errors very easy to locate visually?

Should warnings be highlighted differently from errors? (e.g. a different color)

Under what conditions would you prefer that the error marks disappear? Would you want a way to clear a current error mark without clearing all of them? In other words, you could unmark the errors as you fix them, so that once you deal with each of them, then you know you can try engraving again.

Would it be helpful to have the LilyPond Log widget track with where you are currently editing? That would avoid having to manually scroll through the error list. Just go to the next error mark (manually or by shortcut) and the error line associated with that will be visible in the Log widget. That seems relatively straightforward to implement, since there's already a relationship between each error line in the log and a position in the text.

The only other consumer of the bookmarks system is the Ctrl+B that marks a line, like inserting a break point in some other editors, but without any functionality. How do you use this feature? Or don't you? And would it be more helpful to mark specific selections (a character or multiple lines) instead of only marking by lines?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread. Message ID: @.***>

mr-bronson commented 1 year ago

Thanks for the feedback.

I tend to agree that error marks should not persist after the error log that created them has been lost to Frescobaldi. That means they should never be stored in the meta-data associated with the document and thus never come up in a fresh session, or even the same session if the file is closed and reopened. They should also be cleared (from any referenced files) when the (main) file that created the log is closed (or perhaps even just reloaded).

You say,

They should never persist once the “main” or “master” file associated with it has been ... recompiled

If you mean that this "main" file being compiled was what produced the error marks, I agree. But in your initial example, you had b.ily being accidentally compiled as though it were the main file, thus having its own error log. I don't see that going away, even with a successful compile of a.ly until b.ily is closed (or maybe just reloaded), because in that case the error log is associated with that document, and Frescobaldi doesn't have a good way to know that a.ly included b.ily when it was subsequently compiled successfully. (It only knows for sure about included files based on error messages, but successful includes don't make error messages.)

If, on the other hand, there actually were a problem in b.ily which showed up in a subsequent error log for a.ly, I would agree that the log for b.ily should be tossed along with any error marks it made, even though b.ily was not closed or reloaded. In that case, the log for a.ly clearly supersedes the log for b.ily.

sincere-music commented 1 year ago

Yes, this is tricky and in those cases overlaps with issues concerning how to avoid accidental compilation of .ily files. I would like to forbid Frescobaldi or even LilyPond itself from ever compiling a file with .ily extension, but I'm not sure that's an option. Even better would be for Frescobaldi to understand the structure of projects with include files—we only have a surrogate by indicating master documents manually in a comment.

That's worth considering while defining the scope of this issue here.

Am 22.02.2023 - 00:00 schrieb mr-bronson:

Thanks for the feedback.

I tend to agree that error marks should not persist after the error log that created them has been lost to Frescobaldi. That means they should never be stored in the meta-data associated with the document and thus never come up in a fresh session, or even the same session if the file is closed and reopened. They should also be cleared (from any referenced files) when the (main) file that created the log is closed (or perhaps even just reloaded).

You say,

They should never persist once the “main” or “master” file associated with it has been ... recompiled

If you mean that this "main" file being compiled was what produced the error marks, I agree. But in your initial example, you had b.ily being accidentally compiled as though it were the main file, thus having its own error log. I don't see that going away, even with a successful compile of a.ly until b.ily is closed (or maybe just reloaded), because in that case the error log is associated with that document, and Frescobaldi doesn't have a good way to know that a.ly included b.ily when it was subsequently compiled successfully. (It only knows for sure about included files based on error messages, but successful includes don't make error messages.)

If, on the other hand, there actually were a problem in b.ily which showed up in a subsequent error log for a.ly, I would agree that the log for b.ily should be tossed along with any error marks it made, even though b.ily was not closed or reloaded. In that case, the log for a.ly clearly supersedes the log for b.ily.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread. Message ID: @.***>