DamienCassou / flycheck-hledger

hledger checker for flycheck
GNU General Public License v3.0
13 stars 6 forks source link

Correctly parse errors on Windows #18

Closed shivjm closed 1 year ago

shivjm commented 1 year ago

Errors from hledger weren’t recognized as such using Emacs 30 from Git (2023-05-03) with flycheck-hledger from Git (87ed9d4), hledger 1.30, and flycheck 33snapshot on Windows 10. Instead, I kept getting the errors as ‘Suspicious state’ messages in the minibuffer. Some digging showed the following problems:

  1. The patterns expected hledger: but the output contained hledger.exe:.
  2. The patterns forbade colons in filenames but, on Windows, these will typically contain colons.
  3. A literal \n in a pattern wouldn’t match the line endings (I suppose this is because the output contained \r\n).

This PR fixes all of those issues and, as far as I can tell, correctly parses all the messages in hledger’s error tests. I unfortunately can’t test on a Linux machine at present; I don’t anticipate issues, but it’s always possible the line endings would be matched differently there, in which case perhaps the patterns would need something like (or line-end "\n").

shivjm commented 1 year ago

Switching to anychar alone was fine. I added a test with an hledger message that failed, though:

    :output "hledger.exe: Error: ./file.ledger:2:
  | 2022-01-01
2 |     (a)               1
  |      ^

Strict account checking is enabled, and
account \"a\" has not been declared.
Consider adding an account directive. Examples:

account a
account a    ; type:A  ; (L,E,R,X,C,V)

"

The existing patterns couldn’t handle the line number being on the second line of the excerpt and still parse the message correctly. I fixed this by adding stricter matching for excerpts. All the tests are now passing. (And I’m reasonably confident that if this one Windows case is parsed correctly they should all be.)

shivjm commented 1 year ago

Sorry, you’re definitely right that the commits have got a bit garbled. Let me see why that is.

shivjm commented 1 year ago

I went back to the drawing board and investigated what exactly caused the errors on Windows. Rather than line endings, it seems it was only the filenames with drive letters and colons, which I wasn’t catching in my tests because I used relative paths.

I believe this updated PR should do the trick. I’ve structured it as a pair of atomic commits: one for basic error parsing and one for parsing excerpts where the line number isn’t on the first line.

shivjm commented 1 year ago

Thanks for looking at it. That’s strange about the second commit… I remember splitting up the commits so that the changes to the patterns for the excerpts were in the second commit, but I must have accidentally reverted my changes to the changes along the way. Sorry about that! The second commit should now have the excerpt-related changes that I accidentally put in the first one.

shivjm commented 1 year ago

Thanks for your patience with all the odd Git shenanigans!