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

vim: Update AnyQuotes and AnyBrackets to behave like mini.ai plugin #24845

Conversation

oca159
Copy link
Contributor

@oca159 oca159 commented Feb 14, 2025

Overview

This PR improves the existing mini.ai‐like text-object logic for both “AnyQuotes” (quotes) and “AnyBrackets” (brackets) by adding a multi‐line fallback. The first pass searches only the current line for a best match (cover or next); if none are found, we do a multi‐line pass. This preserves mini.ai's usual “line priority” while ensuring we can detect pairs that start on one line and end on another.

What Changed

  1. Brackets
  • Line-based pass uses gather_line_brackets(map, caret.row()) to find bracket pairs ((), [], {}, <>) on the caret’s line.
  • If that fails, we call gather_brackets_multiline(map) to single‐pass scan the defined region by number of lines, collecting bracket pairs that might span multiple lines.
  • Finally, we apply the mini.ai “cover or next” logic (pick_best_range) to choose the best.
  1. Quotes
  • Similar line-based pass with gather_line_quotes(map, caret.row()).
  • If no local quotes found, we do a multi‐line fallback with gather_quotes_multiline(map), building a big string for the whole buffer and using naive regex for "...", '...', and ....
  • Also preserves “inner vs. outer” logic:
    • For inner (e.g. ciq), we skip bounding quotes or brackets if the range is at least 2 characters wide.
    • For outer (caq), we return the entire range.
  1. Shared “finalize” helpers
  • finalize_bracket_range and finalize_quote_range handle the “inner” skip‐chars vs. “outer” logic.
  • Both rely on the same “line first, then full fallback” approach.

Why This Matters

  • Old Behavior: If you had multi‐line brackets { ... } or multi‐line quotes spanning multiple lines, they weren’t found at all, since we only scanned line by line. That made text objects like ci{ or ciq fail in multi-line scenarios.
  • New Behavior: We still do a quick line pass (for user‐friendly “line priority”), but now if that fails, we do a single‐pass approach across the defined region by number of lines. This detects multi‐line pairs and maintains mini.ai’s “cover‐or‐next” picking logic.

Example Use Cases

  • Curly braces: e.g., opening { on line 10, closing } on line 15 → previously missed; now recognized.
  • Multi‐line quotes: e.g., "'Line 1\nLine 2', no longer missed. We do gather_quotes_multiline with a naive regex matching across newlines.

Tests

  • Updated and expanded coverage in:
    • test_anyquotes_object:
      • Includes a multi-line '...' test case.
      • E.g. 'first' false\nstring 'second' → ensuring we detect multi‐line quotes.
    • test_anybrackets_object:
      • Verifies line‐based priority but also multi‐line bracket detection.
      • E.g., an open bracket ( on line 3, close ) on line 5, which used to fail.

Limitations / Future Enhancements

  • Escaping: The current approach for quotes is naive and doesn’t handle escape sequences (like ") or advanced parser logic. For deeper correctness, we’ll need more advanced logic, this is also not supported in the original mini.ai plugin so it is a known issue that won't be attended for now.

Important Notes

  • Fix for the bug: vim: ci' and ca' motions doesn't work when there is only one quoted string #23889 this PR addresses that bug specifically for the AnyQuotes text object. Note that the issue still remains in the built-in motions (ci', ci", ci`).
  • Caret Position Differences: The caret position now slightly deviates from Vim’s default behavior. This is intentional. I aim to closely mimic the mini.ai plugin. Because these text objects are optional (configurable via vim.json), this adjusted behavior is considered acceptable and in my opinion the new behavior is better and it should be the default in vim. Please review the new tests for details and context.
  • Improved Special Cases: I’ve also refined how “false strings” in the middle and certain curly-bracket scenarios are handled. The test suite reflects these improvements, resulting in a more seamless coding experience overall.
  • The new code utilizes numberOfLines to define the region considered during multiline selection. This approach aligns with the original plugin's implementation. By default, the number of lines is set to 500, but this value can be customized by the user. I chose 500 as the default because it is the configuration used in LazyVim, and it has proven to work effectively in practice.

References:

Thank you for reviewing these changes!

Release Notes:

  • Improve logic of aq, iq, ab and ib motions to work more like mini.ai plugin

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 14, 2025
@zed-industries-bot
Copy link

zed-industries-bot commented Feb 14, 2025

Messages
📖

This PR includes links to the following GitHub Issues: #23889
If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against e6474f0

@oca159
Copy link
Contributor Author

oca159 commented Feb 14, 2025

Hi @ConradIrwin,

I’ve created another version of my original PR: #24167

This version builds on my initial approach, which is inspired by the mini.ai plugin for Neovim. The key difference from my previous PR is that it now incorporates a defined number of lines to scan during multiline selection. This idea is based on the original plugin’s implementation, and I’ve set the default value to 500 lines, as this is the same value used in LazyVim as you can see in the references listed in the PR description.

I believe this version should work well. It’s simpler than the tree-sitter approach, which requires creating custom queries for every language to handle quotes and brackets and sometimes it is tricky and requires custom code to support strings like f-string in python, and strings that start with a symbol like $ and @ in csharp. Additionally, the tree-sitter approach struggles with nested strings. In my opinion, this version is a more robust and maintainable solution.

That said, I’d love to hear your thoughts! If there’s a better way to handle default values for text objects with parameters, I’m happy to adjust accordingly.

Looking forward to your feedback!

@oca159 oca159 force-pushed the ocordova/improve-any-quotes-and-brackets-with-limits branch from 8865a01 to e6474f0 Compare February 14, 2025 03:57
@maxdeviant maxdeviant changed the title vim: update anyquotes and anybrackets to behave like mini.ai plugin (original approach with limited context) vim: Update anyquotes and anybrackets to behave like mini.ai plugin Feb 14, 2025
@maxdeviant maxdeviant changed the title vim: Update anyquotes and anybrackets to behave like mini.ai plugin vim: Update AnyQuotes and AnyBrackets to behave like mini.ai plugin Feb 14, 2025
@ConradIrwin
Copy link
Member

@oca159 nice, thank you!

I still think we should stick with the tree-sitter approach. Though I agree that it's easier to get a first version working manually, we lose out on future improvements (for example the fixes you've added to make the quotes self-consistent improve other tree-sitter based features).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants