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.

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

Bug: Matching brackets not working in plain text. #1108

Closed
heliostatic opened this issue Nov 15, 2021 · 9 comments
Closed

Bug: Matching brackets not working in plain text. #1108

heliostatic opened this issue Nov 15, 2021 · 9 comments
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@heliostatic
Copy link
Contributor

Reproduction steps

Expected: Using mm on an eligible character will go to the matching character in the pair.
Actual: This only seems to work in files with a language server?

2021-11-15.at.16.40.35.-.Alacritty.-.CleanShot.mp4

Environment

  • Platform: macOS
  • Helix version: v0.5.0-114-g225e790
@heliostatic heliostatic added the C-bug Category: This is a bug label Nov 15, 2021
@Omnikar
Copy link
Contributor

Omnikar commented Nov 16, 2021

Parsing of matching brackets is done by Tree Sitter, so mm only works on files with a Tree Sitter grammar (not language server).

@heliostatic
Copy link
Contributor Author

Right, I meant tree sitter, good correction. But I wasn't clear if @archseer was saying it should also work on raw text as well:
Screenshot_20211115-204257.png

@archseer
Copy link
Member

Yeah so mm specifically requires tree-sitter: https://docs.helix-editor.com/keymap.html#match-mode

@sudormrfbin recently labeled all the keybindings that require a LSP or a tree-sitter grammar

@heliostatic
Copy link
Contributor Author

Interesting, thanks!

@heliostatic
Copy link
Contributor Author

Also wondering about why some brackets match in tree-sitter supported files and others don't:

2021-11-16.at.11.57.37.-.Alacritty.-.CleanShot.mp4

@pickfire
Copy link
Contributor

pickfire commented Nov 19, 2021

I think that's because it's not the first and last item in a named node.

https://github.com/ikatyang/tree-sitter-toml/blob/7cff70bbcbbc62001b465603ca1ea88edd668704/grammar.js#L48-L55

    table: $ =>
      seq(
        "[",
        choice($.dotted_key, $._key),
        "]",
        $._line_ending_or_eof,
        repeat(choice($.pair, newline)),
      ),

Our implementation.

let node = match tree
.root_node()
.named_descendant_for_byte_range(byte_pos, byte_pos)
{
Some(node) => node,
None => return None,
};

Notice the you showed that worked because it's the first and last item.

https://github.com/ikatyang/tree-sitter-toml/blob/7cff70bbcbbc62001b465603ca1ea88edd668704/grammar.js#L187-L200

    array: $ =>
      seq(
        "[",
        repeat(newline),
        optional(
          seq(
            $._inline_value,
            repeat(newline),
            repeat(seq(",", repeat(newline), $._inline_value, repeat(newline))),
            optional(seq(",", repeat(newline))),
          ),
        ),
        "]",
      ),

Not very familiar with this, not sure how to solve.

@sudormrfbin
Copy link
Member

sudormrfbin commented Nov 19, 2021

We need an implementation sans tree-sitter anyway for working with plaintext files or files without a grammar, so we could fall back to that if tree-sitter based matching returns no results.

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Dec 1, 2021
@txtyash
Copy link
Contributor

txtyash commented Sep 16, 2022

I too have noticed that mm doesn't work in many places.

@pascalkuthe
Copy link
Member

This was adrwssed by #4288 and shortcoming in bracket matching when TS is available discussed in the comments were addressed I'm a seperate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

8 participants