sideways.vim icon indicating copy to clipboard operation
sideways.vim copied to clipboard

Unbalanced parens make SidewaysArgumentTextobjI/A jump to unrelated line

Open idbrii opened this issue 7 years ago • 4 comments

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 2d5cd2b3cb853067f192cd0e445c35e6007d6fe3 on Gvim 8.0.596 Win64

idbrii avatar Feb 21 '18 19:02 idbrii

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 (https://github.com/AndrewRadev/sideways.vim/blob/01a96f716bad0f7b03043a779e49550139fa2069/plugin/sideways.vim#L39), 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:

  • https://github.com/AndrewRadev/sideways.vim/blob/01a96f716bad0f7b03043a779e49550139fa2069/doc/sideways.txt#L76-L84
  • https://github.com/AndrewRadev/sideways.vim/blob/01a96f716bad0f7b03043a779e49550139fa2069/doc/sideways.txt#L166-L180

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?

AndrewRadev avatar Mar 03 '18 19:03 AndrewRadev

I can confirm that updating to 6703dae2c6b1ea0cc44dbbfd6beafda5b664e071 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 2d5cd2b3cb853067f192cd0e445c35e6007d6fe3, the comment is selected instead of jumps_to_here.

idbrii avatar Mar 05 '18 20:03 idbrii

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.

AndrewRadev avatar Mar 06 '18 07:03 AndrewRadev

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 avatar Mar 29 '18 21:03 idbrii