Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat!: support only "-", "•" as listitem, constrain (argument) definition #97

Merged
merged 6 commits into from
Jun 25, 2023
Merged

feat!: support only "-", "•" as listitem, constrain (argument) definition #97

merged 6 commits into from
Jun 25, 2023

Conversation

justinmk
Copy link
Member

@justinmk justinmk commented Mar 5, 2023 •
edited
Loading

Problem

  • "+" and "*" have many false-positives and aren't commonly used as a listitem prefix.
  • (argument) supports whitespace but,
    • that isn't actually expected by vim's :help syntax definition
    • it causes line-eating error nodes in various cases where a random { is used in text

Solution

  • remove support for "+" and "*" as listitem prefixes
  • disallow whitespace in (argument)

fixes #96

@justinmk justinmk mentioned this pull request Mar 5, 2023
Closed
@clason

This comment was marked as resolved.

Sign in to view
@justinmk

This comment was marked as resolved.

Sign in to view
@clason

This comment was marked as resolved.

Sign in to view
@justinmk justinmk marked this pull request as ready for review May 31, 2023 10:10
justinmk
justinmk commented May 31, 2023
View reviewed changes
grammar.js Outdated Show resolved Hide resolved
@justinmk justinmk marked this pull request as draft June 2, 2023 01:41
justinmk added 2 commits June 2, 2023 09:29
@justinmk
cleanup
6c9251c 
- redundant choice()
- unneeded "NOT listitem" spec: `_li_token` is already defined
  explicitly in terms of space char.
@justinmk
feat!: support only "-", "•" as listitem prefix
afbca92 
"+" and "*" have many false-positives and aren't commonly used as
a listitem prefix.
@justinmk justinmk mentioned this pull request Jun 4, 2023
Closed
@justinmk justinmk mentioned this pull request Jun 21, 2023
Closed
@justinmk justinmk marked this pull request as ready for review June 21, 2023 18:14
@justinmk
Copy link
Member Author

justinmk commented Jun 21, 2023 •
edited
Loading

This is in decent shape now and fixes #96 .

✅ It causes some errors in the generated help, need to double-check that.

justinmk
justinmk commented Jun 21, 2023
View reviewed changes
corpus/arguments.txt
(word))
(word)
(ERROR
(word))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{arg} no longer allows whitespace. That wasn't used in any :help docs anyway, and isn't in vim's help syntax.

justinmk
justinmk commented Jun 21, 2023
View reviewed changes
grammar.js Outdated Show resolved Hide resolved
@justinmk justinmk changed the title feat!: support only "-", "•" as listitem prefix feat!: support only "-", "•" as listitem, constrain (argument) definition Jun 22, 2023
@justinmk justinmk mentioned this pull request Jun 22, 2023
Merged
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 22, 2023
@justinmk
fix(docs): vimdoc syntax errors
7ab28df 
Since neovim/tree-sitter-vimdoc#97
the many cases of *.foo cause parser errors. But even before that, these
were erroneously highlighted as (argument), so fixing them is good.
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 22, 2023
@justinmk
fix(docs): vimdoc syntax errors
c0dd7ed 
Since neovim/tree-sitter-vimdoc#97
the many cases of *.foo cause parser errors. But even before that, these
were erroneously highlighted as (argument), so fixing them is good.
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 22, 2023
@justinmk
fix(docs): vimdoc syntax errors
d931b82 
Since neovim/tree-sitter-vimdoc#97
the many cases of *.foo cause parser errors. But even before that, these
were erroneously highlighted as (argument), so fixing them is good.
@justinmk justinmk mentioned this pull request Jun 25, 2023
Merged
@justinmk justinmk merged commit 755b801 into master Jun 25, 2023
@justinmk justinmk deleted the listitem branch June 25, 2023 16:32
@clason
Copy link
Member

clason commented Jun 25, 2023

any follow-ups planned? otherwise I'd cut a release and bump the bundled parser.

@justinmk
Copy link
Member Author

Yeah I think we should get #103 in before releasing.

@clason
Copy link
Member

clason commented Jun 25, 2023

That one has a bunch of regressions, unfortunately. I'd prefer to tackle this in smaller steps, fwiw. Let's see how it looks after a rebase, though.

@justinmk
Copy link
Member Author

Yeah, no objection to releasing.

@clason
Copy link
Member

clason commented Jun 25, 2023

Release early, release often ;)

(I'm trying to poke tree-sitter into making more regular releases from their parsers; right now, we're either stuck in 2021 -- worst timeline -- or need to track HEAD even for releases.)

In any case, not really urgent; that was meant more as an offer to take things off your plate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing newline in pattern docs
2 participants
@justinmk @clason