BracketHighlighter icon indicating copy to clipboard operation
BracketHighlighter copied to clipboard

Jump/select inside and outside brackets

Open d0vgan opened this issue 9 months ago • 10 comments

At first, thank you very much for the BracketsHighlighter and for continuing to improve it! I have the following feature request:

  • When the caret is outside brackets/quotes, BracketsHighlighter should jump/select outside the brackets/quotes.

Here are a few examples of the desired behavior:

  1. |(abc) becomes (abc)| when jumping (I use the | character to represent the position of the caret);
  2. |(abc) becomes [(abc)] when selecting (I use the [] characters to represent the selection);
  3. { x [(abc)] y } becomes {[ x (abc) y ]} when selecting (again, I use [] to represent the selection).

As can be seen, my request is to preserve the outer position of the caret instead of jumping inside the current brackets.

d0vgan avatar Mar 30 '25 18:03 d0vgan

The request makes sense when "outside adjacent matching" logic is being used. I'll have to look over the code and see if there are any related complications in providing this. It's generally a lower priority issue as without this change, it simply requires the command to be run twice, but I do see how it would seem more intuitive and less cumbersome if it operated in the way you specify.

{ x [(abc)] y } becomes {[ x (abc) y ]} when selecting (again, I use [] to represent the selection).

I do think this behavior already exists as the operation keeps expanding out further. Is there a case where this was not true?

facelessuser avatar Mar 31 '25 18:03 facelessuser

{ x [(abc)] y } becomes {[ x (abc) y ]} when selecting (again, I use [] to represent the selection).

I do think this behavior already exists as the operation keeps expanding out further. Is there a case where this was not true?

Indeed, this one already exists. I just wanted to highlight the fact that when the caret (or selection) is inside (within) an outer pair of brackets, the existing behavior is exactly what is desired, and my feature request applies only to those situations when the caret is outside the brackets.

d0vgan avatar Mar 31 '25 19:03 d0vgan

it simply requires the command to be run twice,

Actually, not always. In all cases similar to the following one: (a = "xyz") The caret firstly jumps inside of the outer () brackets and then it jumps inside of the inner "" quotes and then continues to jump inside "". This definitely differs from jumping outside of the outer () brackets.

d0vgan avatar Jun 19 '25 12:06 d0vgan

Actually, not always. In all cases similar to the following one: (a = "xyz") The caret firstly jumps inside of the outer () brackets and then it jumps inside of the inner "" quotes and then continues to jump inside "". This definitely differs from jumping outside of the outer () brackets.

This is not true. I assume you are talking about selection. So, here's the thing. I'm going to make an assumption, and correct me if I'm wrong. You are using the Example keymap provided to test this?

It looks like I have a typo in the Example keymap. Currently it is this:

    // Select text between brackets
    {
        "no_outside_adj": null,
        "keys": ["ctrl+alt+super+s"],
        "command": "bh_key",
        "args":
        {
            "lines" : true,
            "plugin":
            {
                "command": "bh_modules.bracketselect"
            }
        }
    },

This is a mistake. no_outside_adj should be inside args. This is what I use:

    // Select text between brackets
    {
        "keys": ["ctrl+alt+super+s"],
        "command": "bh_key",
        "args":
        {
            "lines" : true,
            "plugin":
            {
                "command": "bh_modules.bracketselect"
            },
            "no_outside_adj": null
        }
    },

We can see behavior is as expected (when the command is configured as intended). Admittedly, the example is currently wrong.

Image

facelessuser avatar Jun 19 '25 14:06 facelessuser

You'll have to forgive me as it's been a while since I've dug into this code, but now many things are coming back to me.

Originally, this plugin was made for highlighting brackets, there are two modes though: "normal" and "outside adjacent", the latter proving to generally provide the best user experience for highlighting. We made sure it had predictable rules so it always operates in a predictable way.

Selection and navigation are built on top of this, but it is relative to these two modes. We first match a pair, relative to the cursor, with one of the given modes and then navigate or select accordingly.

It was deemed that "outside adjacent", while excellent for highlighting could be cumbersome with navigation and selection as when the cursor is outside a bracket pair, it would still keep choosing that pair in certain situations, as it was adjacent to them, so we could not break free of certain brackets.

Navigation and selection, by default, do not use "outside adjacent" because when I'm in a case such as this (test (test)| test) the cursor keeps targeting the inner bracket.

The case you are running into specifically wants "outside adjacent" because in this case (|(test)) we want the cursor to target the brace as it is outside, but adjacent to bracket. These are conflicting modes. Currently, this case requires you to move your cursor inside and then execute the command.

BracketHighlighter is pretty complex, and we operate on a sliding window and find brackets on demand. The finding of the bracket isn't aware of context, except that it wants to know do I want the brackets with "outside adjacent" logic or normal, inner logic.

To give you the behavior you want, we would need a hybrid mode, but it would need to be aware of the context of the operation. As an example, consider this case (test |(test) test).

  1. If jumping left, we want to target the parent bracket and navigate to the left most bracket.
  2. If we are jumping right, we want to consider the inner parent and jump to the inner right bracket.
  3. If we were highlighting, we'd probably want the inner as well.

The point of explaining this is to highlight that this request is probably far from trivial based on the current design. This hopefully helps to illustrate why it operates as it does.

facelessuser avatar Jun 19 '25 15:06 facelessuser

I use "no_outside_adj": false in the mapping because in case of "no_outside_adj": null the caret does not move anywhere in the situations such as |(...).

I haven't looked into the code of the plugin, but in terms of understanding how it might work, I assume that currently the plugin deals with the position of the caret and decides where to jump basing on the position and the scope. So my idea is to add a boolean variable in addition to the position, having a pair of (position, isOutside). In such case, the value of isOutside will tell whether we should jump inside or outside the pairing bracket. When the caret is already at the bracket character:

  • either outside of it: |( -> (position=N, isOutside=true)
  • or inside of it: (| -> (position=N, isOutside=false)

the plugin should not search for other outer brackets and should immediately jump to the pairing bracket taking isOutside into account. And when the caret is not at the bracket character (for example, | ... ( ... ), then the currently existing algorithm should be applied.

d0vgan avatar Jun 19 '25 16:06 d0vgan

I use "no_outside_adj": false in the mapping because in case of "no_outside_adj": null the caret does not move anywhere in the situations such as |(...).

Yes, this is because brackets are then only matched if the cursor is inside them, meaning if the cursor is outside and adjacent they do not match. Using, false, which allows for "outside adjacent" matching also means you can get stuck in situations where you cannot jump away from the bracket as I explained earlier.

I haven't looked into the code of the plugin, but in terms of understanding how it might work, I assume that currently the plugin deals with the position of the caret and decides where to jump basing on the position and the scope.

The plugin works by first matching a bracket pair based on the relation of the cursor to the bracket and how it is nested within the sliding window. It isn't aware at first if you are highlighting, navigating, or selecting. It just finds the bracket based on "inside" rules or "outside adjacent" rules. Only then is the bracket passed to navigation, selection, or highlighting logic. When the bracket is passed, only that bracket is passed. It is not aware of brackets at higher or lower levels. It is that context that would be needed to suddenly change its mind on what bracket to select. This is just how it is currently designed.

the plugin should not search for other outer brackets and should immediately jump to the pairing bracket taking isOutside into account. And when the caret is not at the bracket character (for example, | ... ( ... ), then the currently existing algorithm should be applied.

I understand what you are asking for, but what I'm telling you is that the current design does not allow this, and it is not trivial to implement. I'm not saying it couldn't be, but I'm setting expectations.

facelessuser avatar Jun 19 '25 17:06 facelessuser

I think the reality is, if we want to make "outside adjacent" as a viable navigation and selection method, we need to adjust things to take in a directionality context, and adjust the algorithm to take that into consideration if/when it is provided. I'm not yet sure about selection, but it would need special considerations as well. Anyway, it is a possibility, but it's not a quick and easy change.

facelessuser avatar Jun 19 '25 18:06 facelessuser

To me it looks like the bracket entity you mentioned, which is passed to another functions, should become a class with such fields as bracketType and isOuter. I believe such a change will require noticeable effort since Python does not have strong typing which might help in this exact case (in strongly-typed languages you simply can't pass an object that does not match the declaration because it will introduce a compilation error).

d0vgan avatar Jun 19 '25 18:06 d0vgan

No, you need to select the correct bracket from the start and adjust the selection relative to it, at least to work within the current framework.

Typing doesn't really make a difference here. I say this as someone who has developed in C/C++ as well. Whether objects are used or not is neither here nor there; with and without class objects are both viable options. Whatever is done would ideally minimize the disruptiveness to the code base, but I'm not against overhauling things if it is absolutely necessary. But yes, it is not a simple change.

facelessuser avatar Jun 19 '25 18:06 facelessuser