-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Fix LaTeX bug + add tests #96
Conversation
@alstr It doesn't look like I can add you as a reviewer on this, but just wanted to comment to make sure you're aware of it. Cheers! |
Thanks very much for the work on this, I'll take a look as soon as I'm able! |
And yes, the tests should not depend on the remote |
@alstr - I just added tests for Julia / AutoHotKey / Org Mode / Handlebars, which are the ones listed as outstanding. Worth noting that the Julia test is failing because it creates 3 issues instead of 2, which I think has something to do with the actual parsing logic, but I don't have bandwidth to dig into it. I propose that if the tests work for you that we merge this to fix the LaTeX issue (which is actively blocking some of my personal workflows) and then move the Julia parsing error to a new issue / PR. Does that sound good to you? |
1. Swap `\` for `\\` for TeX block comments to avoid issues with parsing 2. Add tests for TeX, Julia, AutoHotKey, Org mode, and Handlebars 3. Ensure tests can be run with 1 line command 4. Change logic in test framework so that tests run off of local version of `syntax.json` rather than remote version 5. Add details for running tests + adding tests for your syntax PR to readme 6. Add .gitignore file for python (to avoid compiles form tests winding up in commits)
index 0000000..7cccc5b | ||
--- /dev/null | ||
+++ b/src/tests/example_file.jl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alstr FYI this is the diff for the julia failing test (the add/remove diffs are the same actual julia content).
Not quite sure why this would be parsed into 3 todos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's because the line comment marker #
appears within the block comment marker #=
/=#
. I haven't tested this but I've opened issue #102. Probably need to only match a marker if it is an exact match.
@alstr friendly ping on this! This PR is defo far from perfect, but want to get at the very least the TeX portion fixed sooner rather than later if possible. Thanks! |
Thanks very much! |
This PR aims to
I was able to identify this by adding a local change to swap
\begin
=>\\begin
and\end
=>\\end
in the regex parts for LaTeX before parsing. However, not all tests pass after doing this (which appears to be due to other underlying issues with the tests).Also worth noting that the tests right now depend explicitly on the version of
syntax.json
which is on master of this repo, which makes it painful to test locally. I overrode that in the tests file to make them easier to work with in the future