import: fix scoping problems with match groups

simonmichael / hledger

Robust, fast, intuitive plain text accounting tool with CLI, TUI and web interfaces.

https://hledger.org

GNU General Public License v3.0

2.97k stars 317 forks source link

import: fix scoping problems with match groups #2179

Closed jmtd closed 7 months ago

jmtd commented 7 months ago

This is a beginning of an attempt to fix #2158. First a test to capture the problem:

    issue-2158:                   FAIL
      /home/jon/git/hledger/hledger-lib/Hledger/Read/RulesReader.hs:1547:
      expected: Just "account:Text 1:Text 2"
       but got: Just "account:Text 1 - Text 2:Text 1"

I'm not sure why the order of rconditionalblocks appears to be reversed from the order in rules files. That's where I'll look when I next pick this up.

jmtd commented 7 months ago

I'm not sure why the order of rconditionalblocks appears to be reversed from the order in rules files.

Probably relevant: https://github.com/simonmichael/hledger/blob/master/hledger-lib/Hledger/Read/RulesReader.hs#L457

jmtd commented 7 months ago

I'm not sure why the order of rconditionalblocks appears to be reversed from the order in rules files. That's where I'll look when I next pick this up.

Red herring. The parser code prepends ConditionalBlocks as it parses them (presumably for efficiency); at the end of parsing mkrules reverses the list.

jmtd commented 7 months ago

I think this is ready for review.

[ ] I've had to (temporarily?) adjust regexMatchValue to use rconditionalblocks rather than rblocksassigning, as I'm not sure yet how to update that field to reflect an altered set of ConditionalBlocks
[ ] Maybe (Either FieldTemplate ConditionalBlock) is a bit of a mouthful. We could introduce a type alias for the Either bit. On the other hand, the function is now only used in two places, so perhaps that's overkill.
[ ] I don't know how legible hledgerField and hledgerFieldValue are. I've been staring at them too long to judge right now.

simonmichael commented 7 months ago

Thanks for the fix @jmtd!

(I tried to merge keeping history, probably made a mess as usual..)

alerque commented 7 months ago

Thanks for the work on this! I'm looking forward to being able to use it.

Which reminds me I need to get that Nix Flake working so I can run arbitrary versions in production...