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

Unbalanced parens make SidewaysArgumentTextobjI/A jump to unrelated line #24

Open
idbrii opened this issue Feb 21, 2018 · 4 comments
Open

Comments

@idbrii
Copy link

idbrii commented Feb 21, 2018

Several times I've had sideways select text that doesn't include my cursor position. I think it would make more sense to do nothing than jump to another part of the buffer. I've found a reliable and small repro case:

EditorGUI.BeginProperty(position, label, property);
{
    SerializedProperty joint = property.serializedObject.FindProperty("Bone"); // will jump to here

    Debug.Log(string.Format("1) Found a good one: {0}"), joint); // selecting from here
}
EditorGUI.EndProperty();

Do:

/joint)
vi,

(i, is SidewaysArgumentTextobjI)

Result:

The SerializedProperty line is selected.

va, selects the S in SerializedProperty and the space before it.

The only case I can think of where sideways selects something that doesn't include my cursor position is on a comma and the next argument is on the following line (but I'd argue it should select the preceeding argument). Would it make sense to limit the textobjs to require inclusion of the cursor position?

Using 2d5cd2b on Gvim 8.0.596 Win64

@AndrewRadev
Copy link
Owner

There's some interesting layers to the problem here.

In general, the plugin couldn't handle unbalanced brackets. It expects that an expression is valid, because I honestly don't know how I'd handle the logic for it otherwise :). That said, the problem here is that the bracket is in a string, and that should be ignored by default.

The thing is, I ignore particular syntax groups for ruby (

\ 'skip_syntax': ['rubyString', 'rubySymbol', 'rubyComment', 'rubyInterpolation'],
), but this is a Java example (I think), which uses a global definition.

So, first off, I fixed a few places that weren't correctly skipping syntax patterns. I also added some default behaviour to try to ignore all "Comment" and "String" patterns, with a setting to escape it if it's necessary. The relevant docs are here:

  • Syntax skipping ~
    The plugin will try to be smart and ignore text in any syntax groups that
    include "Comment" and "String". Some filetype-specific definitions have a
    bigger list of groups, see the "skip_syntax" sections of the definitions.
    If you'd like to skip specific syntax groups (or not skip anything at all),
    you can use the |b:sideways_skip_syntax| buffer-local variable. See below in
    |sideways-settings| for detailed instructions.
  • *b:sideways_skip_syntax*
    >
    let b:sideways_skip_syntax = ['javaString']
    <
    Default value: ['Comment', 'String']
    This is a buffer-local setting, which means you should set it per-filetype.
    For instance, if you wanted to skip particular syntax groups in the java
    filetype, you'd put the let-clause in the file ~/.vim/ftplugin/java.vim.
    This specifies the syntax groups that the plugin will skip over when looking
    for matching brackets and for the starts of lists. It should be a list of
    syntax groups (or rather, parts of syntax groups -- the names are matched with
    the |=~#| operator). By default, the plugin skips "Comment" and "String"
    syntax items.

Long story short, the example you showed should work as expected now, but you should consider the relevant documentation in case you run into similar issues that I haven't thought of.

Does this seem to solve the problem?

@idbrii
Copy link
Author

idbrii commented Mar 5, 2018

I can confirm that updating to 6703dae fixes my C# example above. Thanks so much!

However, I experience some similar jumping to far away text (also C#):

if (true)
{
    Debug.LogError(
            "",
            sdfsf,
            children.Length, // selected without deleteme
            joint);
    return null;
}
else
{
    GUI.Label(position,
            "deleteme",
            jumps_to_here);
}

And

/joint)
vi,

The jumps_to_here text is selected instead of joint.

Alternatively, minor modifications cause the comment to be selected instead of joint:

  • Removing the first string or making it nonempty
  • Removing the deleteme line
  • Removing the whole else block

Removing the first string and the comment makes it behave as expected.

Possibly this is related to your changes. On 2d5cd2b, the comment is selected instead of jumps_to_here.

@AndrewRadev
Copy link
Owner

The jumps_to_here text is selected instead of joint.

Hm, something to do with the string matching, I think. I've adjusted one of the search patterns, and it seems to work now, for this example.

Alternatively, minor modifications cause the comment to be selected instead of joint:

This one is going to be trickier. I'll need some time to think it over. Comments are ignored mostly to avoid code that's competely commented out. A line-based comment in an argument list is something I hadn't considered.

@idbrii
Copy link
Author

idbrii commented Mar 29, 2018

On 01bcf0f, sideways is better behaved but not correct on my example (copypasted to new cs file). It selects the comment instead of jumps_to_here.

If sideways uses syntax then you should know I'm using omnisharp vim to improve vim's C# syntax.

idbrii added a commit to idbrii/daveconfig that referenced this issue Mar 30, 2018
Improves my pathological case:
AndrewRadev/sideways.vim#24 (comment)

Changelog:
Try to fix matching empty quotes
Fix text object, wrong column used
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