Skip to content

Conversation

@Neko-Box-Coder
Copy link
Contributor

Escape braces that are in string or comments when finding matching brace.

To test this, just a filetype that contains brace ({}) in string or comment.

My test case:

//Test { } test

if (true)
{
    {std::string testString = "this is { a test string"}
}

@Neko-Box-Coder Neko-Box-Coder force-pushed the EscapeBracesInStringOrComments branch from 9ace8e7 to 8cd69a1 Compare July 6, 2024 20:28
@Neko-Box-Coder Neko-Box-Coder force-pushed the EscapeBracesInStringOrComments branch from 8cd69a1 to 1ea20dc Compare July 6, 2024 20:29
Copy link
Contributor

@Andriamanitra Andriamanitra left a comment

Choose a reason for hiding this comment

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

Some stuff will still slip through (for example regex literals in JS/Ruby/etc) but I don't think there's a good way to handle that (at least without going through all syntaxes and coming up with a consistent way to detect this kind of thing). Just skipping strings and comments probably covers >95% of the circumstances where this is a problem.

Comment on lines 1144 to 1165
func (b *Buffer) getSortedSyntaxIndices(lineN int) []int {
keys := make([]int, 0, len(b.Match(lineN)))
for k := range b.Match(lineN) {
keys = append(keys, k)
}
sort.Ints(keys)
return keys
}

// Returns the Group (syntax highlight group ID) at the specified location and a boolean
// that indicates if a group is found or not
func (b *Buffer) GetGroupAtLoc(loc Loc) (highlight.Group, bool) {
sortedIndices := b.getSortedSyntaxIndices(loc.Y)
i := sort.SearchInts(sortedIndices, loc.X)
if i == 0 || i == len(sortedIndices) {
return 0, false
}
if sortedIndices[i] == loc.X && b.Match(loc.Y)[sortedIndices[i]] != 0 {
return b.Match(loc.Y)[sortedIndices[i]], true
}
return b.Match(loc.Y)[sortedIndices[i - 1]], true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something but is the sorting necessary?

Suggested change
func (b *Buffer) getSortedSyntaxIndices(lineN int) []int {
keys := make([]int, 0, len(b.Match(lineN)))
for k := range b.Match(lineN) {
keys = append(keys, k)
}
sort.Ints(keys)
return keys
}
// Returns the Group (syntax highlight group ID) at the specified location and a boolean
// that indicates if a group is found or not
func (b *Buffer) GetGroupAtLoc(loc Loc) (highlight.Group, bool) {
sortedIndices := b.getSortedSyntaxIndices(loc.Y)
i := sort.SearchInts(sortedIndices, loc.X)
if i == 0 || i == len(sortedIndices) {
return 0, false
}
if sortedIndices[i] == loc.X && b.Match(loc.Y)[sortedIndices[i]] != 0 {
return b.Match(loc.Y)[sortedIndices[i]], true
}
return b.Match(loc.Y)[sortedIndices[i - 1]], true
}
func (b *Buffer) GetGroupAtLoc(loc Loc) (highlight.Group, bool) {
found := -1
for i := range b.Match(loc.Y) {
if i > found && i <= loc.X {
found = i
}
}
if found == -1 {
return 0, false
}
return b.Match(loc.Y)[found], true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The suggested change won't work when the cursor is at the end of a syntax change, which has an empty group and needs the previous group id.

The sorted indices are now being reused for the rest of the lines without iterating the whole line again, which will be useful for extreme cases where the line is very long, for example, minified js or json.

Copy link
Contributor

Choose a reason for hiding this comment

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

The suggested change won't work when the cursor is at the end of a syntax change, which has an empty group and needs the previous group id.

Can you provide an example of this / how to test it? It should be fixable – if the only reason you need the sorted indices is to call sort.SearchInts afterwards you can always do it with a simple loop (less code and no allocations).

@dmaluka
Copy link
Collaborator

dmaluka commented Jul 7, 2024

I doubt this is a good idea, at least because:

  1. It prevents jumping to a (correct) matching brace within the same string or comment. (So it "fixes" one thing by breaking another.)
  2. It doesn't work when syntax highlighting is disabled (syntax option is off).

So, this is an ad-hoc heuristic, and while it is useful for cases like if c == "(", its usefulness is quite limited, and I doubt the complexity and inconsistency it introduces is worth it.

@Neko-Box-Coder
Copy link
Contributor Author

@dmaluka
Yeah fair enough. I can see your points.

Would it work if I keep FindOpeningBrace() and FindClosingBrace() (without the braces escape logic), and expose these functions as bindable actions.

@dmaluka
Copy link
Collaborator

dmaluka commented Jul 7, 2024

Would it work if I keep FindOpeningBrace() and FindClosingBrace() (without the braces escape logic), and expose these functions as bindable actions.

What would be their difference from the existing JumpToMatchingBrace action?

@Neko-Box-Coder
Copy link
Contributor Author

@dmaluka
The cursor doesn't have to be on a brace. It can go to the corresponding braces when inside a scope block.
I always wanted it but never got time to do it. I don't think we have something like that at the moment right?

@dmaluka
Copy link
Collaborator

dmaluka commented Jul 7, 2024

Ok, that was discussed in #3308 (#3308 (comment) and below). I provided a Lua implementation for that in #3308 (comment).

Yeah, it might make sense to implement those jumpToNextUnpairedBraceLeft and jumpToNextUnpairedBraceRight as micro's built-in actions.

@Neko-Box-Coder
Copy link
Contributor Author

The proposed logic for jumping to opening and closing brace is now in #3384
Closing this one instead.

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

Successfully merging this pull request may close these issues.

3 participants