durableOne / orgmunge

MIT License
75 stars 4 forks source link

tests : failing tests - 1) non-standard character not parsed, 2) TODO in title triggers a syntax error, 3) a title with just a tag triggers an error #12

Closed crdoconnor closed 1 year ago

crdoconnor commented 1 year ago

I ran into these bugs when I was parsing some of my own org files.

1) Non-standard characters. They mostly seemed to be triggered by text that I had copied from either a slack message or an email into my org mode files.

2) The second test is about TODO appearing in the title. This test passes for some reason, but it definitely triggered a syntax error when I tried to parse it. I am not sure why this is.

3) The third is a title with only a tag.

I'm following the pattern I set up before - testing that a string gets roundtripped properly.

This PR doesn't contain the fix, I'm afraid, but I figured you'd probably want to see the bugs.

durableOne commented 1 year ago

Thanks. I will take a look. I can tell you that bug 2 started appearing because now the default todo keyword is "TODO" so it can't appear after the title because the parser expects the todo keywords to appear before the title. The easiest solution is to pass "fake" todo keywords when parsing the file:

Org('my/file.org', todos={'todo_states': {'todo': "TDO"}, 'done_states': {'done': "DNE"}})
crdoconnor commented 1 year ago

Yes, I just realized that this morning as I was brushing my teeth. I think if the default todo keyword is set to TODO it should start invoking the bug.

Thanks for taking a look.

On Thu, Nov 16, 2023 at 2:01 PM Joe Riad @.***> wrote:

Thanks. I will take a look. I can tell you that bug 2 started appearing because now the default todo keyword is "TODO" so it can't appear after the title because the parser expects the todo keywords to appear before the title. The easiest solution is to pass "fake" todo keywords when parsing the file:

Org('my/file.org', todos={'todo_states': {'todo': "TDO"}, 'done_states': {'done': "DNE"}})

— Reply to this email directly, view it on GitHub https://github.com/durableOne/orgmunge/pull/12#issuecomment-1814491606, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNLCKEYDV2Y4DCZAYF3YEYMDVAVCNFSM6AAAAAA7LKGR6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUGQ4TCNRQGY . You are receiving this because you authored the thread.Message ID: @.***>

durableOne commented 1 year ago

All the tests are now passing as of https://github.com/durableOne/orgmunge/commit/cc5fa46618653bc024d27722ff4102829f9abc52

  1. Empty titles are now supported
  2. Titles can now have todo keywords in them
  3. Some of the tests were failing because the assertion was comparing the original input string to the stringified version of the parsed tree. The problem is that the parser doesn't preserve whitespace. I have modified the failing cases to remove extraneous whitespace (or add more spaces before the tags in the test case with the empty title) to get the tests to pass. In the future, a better way to write assertions to avoid this is to compare the parsed headings to functionally equivalent headings that you construct using the Heading class.
crdoconnor commented 1 year ago

That's amazing. Thank you. I'm gonna try it out now.

crdoconnor commented 1 year ago

Working great :partying_face: :fireworks: