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

JumpToMatchingBrace inconsistent with adjacent unpaired braces e.g. )[ or }( or )( #3308

Closed
Gavin-Holt opened this issue May 21, 2024 · 12 comments

Comments

@Gavin-Holt
Copy link

Gavin-Holt commented May 21, 2024

Hi,

I am having trouble with JumpToMatchingBrace action in my windows console. My console has a block cursor, I expect the cursor to bounce back and forth between one brace and its partner. At any point pressing delete should delete the brace i.e. the cursor is placed upon the brace.

With the current implementation the cursor moves to the right of the found brace, and if this character happens to also be a brace, repeating the action does not return to the original brace - it goes off searching downstream.

For example in the following line of Lua, clicking upon the first [, and executing JumpToMatchingBrace, will take my cursor to the first (. Executing JumpToMatchingBrace again will take the cursor past the first ) . From here further executions bounce between the first ( and a position just past the first ).

  local success = bp["JumpToMatchingBrace"](bp)

My guess is that the code has been written while watching an I beam cursor. It still malfunctions as above, but looks pretty with the cursors inside the braces. To function properly the I beam cursor always needs to locate on the left of the brace.

I have attempted to rewrite JumpToMatchingBrace in Lua, but manipulating the userdata locations is confusing.

I am similarly frustrated after several attempts to write a selectinner Lua function to select the contents of matching braces.

A nice tutorial about manipulating cursor/match location userdata would be greatly appreciated,

Kind Regards Gavin Holt

@Gavin-Holt
Copy link
Author

Hi,

On the subject of braces; it would be nice to bind a key so the cursor jumps to the matching brace (if the cursor is on a brace), or finds the next unpaired brace if the cursor is not on a brace. e.g.

    "Ctrl-J": "JumpToMatchingBrace | JumpToNextUnpairedBrace",

A JumpToNextUnpairedBrace action would be the basis for a selectinner function (CTRL-Shift-I) , and the trivial extension to a selectouter function (CTRL-Shift-J).

Kind Regards Gavin Holt

@Gavin-Holt Gavin-Holt changed the title JumpToMatchingBrace inconsistent with adjacent unpaired braces e.g. )[ or }( JumpToMatchingBrace inconsistent with adjacent unpaired braces e.g. )[ or }( or )( May 22, 2024
@dmaluka
Copy link
Collaborator

dmaluka commented May 22, 2024

I remember we were investigating the reasons for this "feature"... Here is a summary: #2876 (comment)

@Gavin-Holt
Copy link
Author

Gavin-Holt commented May 23, 2024

Hi,

Thank you for pointing me to the previous discussions.

From my perspective matchbrace highlighting option and JumpToMatchingBrace action should only apply to the character under the (block) cursor (i.e to the right of the I beam cursor).

I would really value a JumpToNextUnpairedBrace action, to aid selecting all the contents of braces (including any nested braces).

Not wanting to impose upon others - could these be written in Lua? The only drawback would be that a Lua function would not work well with multiple cursors so built-in actions may be better.

Kind Regards Gavin Holt

@dmaluka
Copy link
Collaborator

dmaluka commented May 23, 2024

I don't quite understand what exact behavior of JumpToNextUnpairedBrace do you want. Could you explain it with some examples?

For example, if the text is (foo(bar)baz), to which brace should JumpToNextUnpairedBrace jump if the cursor is on foo, if it is on bar and if it is on baz?

@Gavin-Holt
Copy link
Author

Gavin-Holt commented May 23, 2024

Hi

I really want a selectinner action, which I believe is dependent upon the ability to JumpToNextUnpairedBrace.

For your test string, (foo(bar)baz) a selectinner action would select the content of the nested brace if the (block) cursor was on any italic character, or select the whole brace contents if the (block) cursor was on a bold character.

For example:

  • If the cursor was on the f, o, second b, second a or the z the resulting selection would be foo(bar)baz.
  • If the cursor was on the first b, first a or the r the resulting selection would be bar.
  • If the cursor was on a bold brace the resulting selection would be foo(bar)baz.
  • If the cursor was on an italic brace the resulting selection would be bar.

To achieve this I would first search for the next unpaired brace, save this location, then jump to matching brace, then select from this location to the saved location - excluding the limiting braces.

Searching for the next unpaired brace would require counting opening/closing braces (by type), and finding the first closing brace without a preceding opening brace. If the cursor is initially on a closing brace then this is the found position and the cursor does not move forward.

A selectouter action would be very similar but include the limiting braces in the selection.

Kind Regards Gavin Holt

@Gavin-Holt
Copy link
Author

Gavin-Holt commented May 24, 2024

Hi

I would plan to use all of these new actions, in key bindings:

      "CtrlJ": " JumpToMatchingBrace | JumpToNextUnpairedBrace ",
      "CtrlShiftJ": "selectinner",

With easy shortcuts to navigate braces - we might even attract some Lisp coders. 😊

Kind Regards Gavin Holt

@dmaluka
Copy link
Collaborator

dmaluka commented May 24, 2024

Got it. So, actually, it would be not just one new action JumpToNextUnpairedBrace but two separate actions, since it makes difference in which direction to jump. Precisely, it would be like:

  • JumpToNextUnpairedBraceLeft: jump to the nearest opening brace to the left of the cursor, that has no matching closing brace that is also to the left of the cursor.
  • JumpToNextUnpairedBraceRight: jump to the nearest closing brace to the right of the cursor, that has no matching opening brace that is also to the right of the cursor.

If we forget for a moment about those actions and focus just on your selectinner, its algorithm would be like:

  1. If the cursor in on a brace: try to find a matching brace, and if found, select the text between the two braces.
  2. Otherwise:
    2.1. Try to find the nearest opening brace to the left of the cursor, that has no matching closing brace that is also to the left of the cursor.
    2.2. If found, try to find a maching brace for it, and if found, select the text between the two braces.

I might try to code up some Lua code doing that for you, once I have some time... Or in the meantime, I might suggest you try using Loc's Move() method:

the next character to the right of the given location:

loc:Move(1, bp.Buf)

or the next character to the left of it:

loc:Move(-1, bp.Buf)

It automatically takes care of whether this next character is on the same line or on the next/previous line.

You might also look how SelectWord() is implemented, and try to do something similar (unless you already did?).

@dmaluka
Copy link
Collaborator

dmaluka commented May 25, 2024

Ok... I think what you need for your selectinner and selectouter is not Jump* actions but FindMatchingBrace() function. For example, if you found an "unpaired" brace, you shouldn't jump to it right away, you should first check if the matching brace for it exists (otherwise, there is nothing to select, right?). So you could use FindMatchingBrace() to find the matching brace for the given location without jumping to it, and regardless of the current cursor location.

But, FindMatchingBrace() is very inconvenient to use from Lua. But I have some ideas how to rework its interface and implementation to streamline its usage:

  1. Instead of passing a single brace pair to FindMatchingBrace(), make it traverse all brace pairs in BracePairs on its own.
  2. Add a boolean parameter to FindMatchingBrace() for disabling the "I-beam" behavior, i.e. for finding the matching brace always for the brace precisely at the given location, not one character left of it.

As a bonus, it would allow to fix the minor issue I described in #2876 (comment), since it would allow FindMatchingBrace() to prioritize between different braces.

@dmaluka
Copy link
Collaborator

dmaluka commented May 26, 2024

So... I've uploaded #3319 which improves FindMatchingBrace() almost like I suggested above. With this PR you can use the following custom Lua action, instead of JumpToMatchingBrace, to jump exactly to the matching brace for the current cursor location (e.g. without the I-beam behavior):

function jumpToExactMatchingBrace(bp)
    local mb, left, found = bp.Buf:FindMatchingBrace(-bp.Cursor.Loc)
    if found and not left then
        bp.Cursor:GotoLoc(mb)
        bp:Relocate()
        return true
    end
    return false
end

Then, use can also use these functions for finding the next "unpaired" brace to the left or to the right:

function findNextUnpairedBraceLeft(bp)
    local countPar = 0
    local countCurl = 0
    local countSq = 0

    local loc = -bp.Cursor.Loc
    local bufStart = bp.Buf:Start()

    while loc:GreaterThan(bufStart) do
        loc = loc:Move(-1, bp.Buf)
        local curLine = bp.Buf:Line(loc.Y)

        local r = util.RuneAt(curLine, loc.X)
        if r == "(" then
            countPar = countPar + 1
            if countPar > 0 then
                return loc
            end
        elseif r == "{" then
            countCurl = countCurl + 1
            if countCurl > 0 then
                return loc
            end
        elseif r == "[" then
            countSq = countSq + 1
            if countSq > 0 then
                return loc
            end
        elseif r == ")" then
            countPar = countPar - 1
        elseif r == "}" then
            countCurl = countCurl - 1
        elseif r == "]" then
            countSq = countSq - 1
        end
    end
    return nil
end

function findNextUnpairedBraceRight(bp)
    local countPar = 0
    local countCurl = 0
    local countSq = 0

    local loc = -bp.Cursor.Loc
    local bufEnd = bp.Buf:End()

    while loc:LessThan(bufEnd) do
        loc = loc:Move(1, bp.Buf)
        local curLine = bp.Buf:Line(loc.Y)

        local r = util.RuneAt(curLine, loc.X)
        if r == ")" then
            countPar = countPar + 1
            if countPar > 0 then
                return loc
            end
        elseif r == "}" then
            countCurl = countCurl + 1
            if countCurl > 0 then
                return loc
            end
        elseif r == "]" then
            countSq = countSq + 1
            if countSq > 0 then
                return loc
            end
        elseif r == "(" then
            countPar = countPar - 1
        elseif r == "{" then
            countCurl = countCurl - 1
        elseif r == "[" then
            countSq = countSq - 1
        end
    end
    return nil
end

And you use them to implement the actual actions for jumping to the next "unpaired" brace:

function jumpToNextUnpairedBraceLeft(bp)
    local loc = findNextUnpairedBraceLeft(bp)
    if loc ~= nil then
        bp.Cursor:GotoLoc(loc)
        bp:Relocate()
        return true
    end
    return false
end

function jumpToNextUnpairedBraceRight(bp)
    local loc = findNextUnpairedBraceRight(bp)
    if loc ~= nil then
        bp.Cursor:GotoLoc(loc)
        bp:Relocate()
        return true
    end
    return false
end

Finally, you can implement selectin and selectout:

function selectin(bp)
    local braceStart = findNextUnpairedBraceLeft(bp)
    if braceStart ~= nil then
        local braceEnd, left, found = bp.Buf:FindMatchingBrace(braceStart)
        if found and not left then
            braceStart = braceStart:Move(1, bp.Buf)
            if braceStart:LessThan(braceEnd) then
                bp.Cursor:SetSelectionStart(braceStart)
                bp.Cursor:SetSelectionEnd(braceEnd)
                bp.Cursor.OrigSelection = -bp.Cursor.CurSelection
                bp.Cursor:GotoLoc(braceEnd)
                bp:Relocate()
                return true
            end
        end
    end
    return false
end

function selectout(bp)
    local braceStart = findNextUnpairedBraceLeft(bp)
    if braceStart ~= nil then
        local braceEnd, left, found = bp.Buf:FindMatchingBrace(braceStart)
        if found and not left then
            braceEnd = braceEnd:Move(1, bp.Buf)
            if braceStart:LessThan(braceEnd) then
                bp.Cursor:SetSelectionStart(braceStart)
                bp.Cursor:SetSelectionEnd(braceEnd)
                bp.Cursor.OrigSelection = -bp.Cursor.CurSelection
                bp.Cursor:GotoLoc(braceEnd)
                bp:Relocate()
                return true
            end
        end
    end
    return false
end

@Gavin-Holt
Copy link
Author

Hi,

Many thanks, I will enjoy studying these new functions.
My investigations will need to wait for PR merge and then Nightly binary release.
Meantime I have some generic goto/select functions using regex to share:

function selectforwards(Current)
-- Select forwards from cursor using regex (set to ungreedy and case insensitive)
-- this does require escaping of regex metacharacter
-- this does not pollute the find history
-- this does not highlight all matches
    local prompt = "SelectUntil : "
    local seed = "(?sU-i).*"
    local history = "SelectUntil"
    micro.InfoBar():Prompt(prompt, seed, history,
        function(input)
            return
        end,
        function(input, canceled)
            if input and not canceled then
                local top = Current.Buf:Start()
                local bottom = Current.Buf:End()
                local searchLoc = -Current.Cursor.Loc
                local down = true
                local useRegex = true
                local res, found = Current.Buf:FindNext(input, top, bottom, searchLoc, down, useRegex)
                if found then
                    Current.Cursor:SetSelectionStart(searchLoc)
                    Current.Cursor:SetSelectionEnd(res[2])
                    Current.Cursor.OrigSelection[1] = -Current.Cursor.CurSelection[1]
                    Current.Cursor.OrigSelection[2] = -Current.Cursor.CurSelection[2]
                    Current.Cursor:GotoLoc(res[2])
                    Current:Relocate()
                end
            end
        end
    )
end
function selectbackwards(Current)
-- Select backwards from cursor using regex (set to ungreedy and case insensitive)
-- this does require escaping of regex metacharacter
-- this does not pollute the find history
-- this does not highlight all matches
    local prompt = "SelectBack : "
    local seed = "(?sU-i)"
    local history = "SelectBack"
    micro.InfoBar():Prompt(prompt, seed, history,
        function(input)
            return
        end,
        function(input, canceled)
            if input and not canceled then
                local top = Current.Buf:Start()
                local bottom = Current.Buf:End()
                local searchLoc = -Current.Cursor.Loc
                local down = false
                local useRegex = true
                local res, found = Current.Buf:FindNext(input, top, bottom, searchLoc, down, useRegex)
                if found then
                    Current.Cursor:SetSelectionStart(searchLoc)
                    Current.Cursor:SetSelectionEnd(res[1])
                    Current.Cursor.OrigSelection[1] = -Current.Cursor.CurSelection[1]
                    Current.Cursor.OrigSelection[2] = searchLoc
                    Current.Cursor:GotoLoc(res[1])
                    Current:Relocate()
                end
            end
        end
    )
end
function goforwards(Current)
-- Move forwards from cursor using regex (set to ungreedy and case insensitive)
-- this does require escaping of regex metacharacter
-- this does not pollute the find history
-- this does not highlight all matches
-- this will alight at the beginning of a multicharacter search
    local prompt = "Go forwards : "
    local seed = "(?sU-i)"
    local history = "Goforwards"
    micro.InfoBar():Prompt(prompt, seed, history,
        function(input)
            return
        end,
        function(input, canceled)
            if input and not canceled then
                local top = Current.Buf:Start()
                local bottom = Current.Buf:End()
                local searchLoc = -Current.Cursor.Loc
                local down = true
                local useRegex = true
                local res, found = Current.Buf:FindNext(input, top, bottom, searchLoc, down, useRegex)
                if found then
                    Current.Cursor:SetSelectionStart(res[1])
                    Current.Cursor:SetSelectionEnd(res[2])
                    Current.Cursor.OrigSelection[1] = -Current.Cursor.CurSelection[1]
                    Current.Cursor.OrigSelection[2] = -Current.Cursor.CurSelection[2]
                    Current.Cursor:GotoLoc(res[2])
                    Current:Relocate()
                    Current.Cursor:Deselect(true)
                end
            end
        end
    )
end
function gobackwards(Current)
-- Move backwards from cursor using regex (set to ungreedy and case insensitive)
-- this does require escaping of regex metacharacter
-- this does not pollute the find history
-- this does not highlight all matches
-- this will alight at the beginning of a multicharacter search
    local prompt = "Go backwards : "
    local seed = "(?sU-i)"
    local history = "Gobackwards"
    micro.InfoBar():Prompt(prompt, seed, history,
        function(input)
            return
        end,
        function(input, canceled)
            if input and not canceled then
                local top = Current.Buf:Start()
                local bottom = Current.Buf:End()
                local searchLoc = -Current.Cursor.Loc
                local down = false
                local useRegex = true
                local res, found = Current.Buf:FindNext(input, top, bottom, searchLoc, down, useRegex)
                if found then
                    Current.Cursor:SetSelectionStart(res[1])
                    Current.Cursor:SetSelectionEnd(res[2])
                    Current.Cursor.OrigSelection[1] = -Current.Cursor.CurSelection[1]
                    Current.Cursor.OrigSelection[2] = -Current.Cursor.CurSelection[2]
                    Current.Cursor:GotoLoc(res[2])
                    Current:Relocate()
                    Current.Cursor:Deselect(true)
                end
            end
        end
    )
end

I find these functions very useful, much quicker than reaching for the mouse.

Kind Regards Gavin Holt

PS I will leave this open as a point for discussion - until the PR is merged

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 6, 2024

FYI #3319 merged.

@Gavin-Holt
Copy link
Author

Hi,

New binary + your functions work beautifully.

Many Thanks

Kind Regards Gavin Holt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants