AndrewRadev / sideways.vim

A Vim plugin to move function arguments (and other delimited-by-something items) left and right.
http://www.vim.org/scripts/script.php?script_id=4171
MIT License
479 stars 9 forks source link

:Sideways* failed when `multiline lists` #39

Closed Shane-XB-Qian closed 4 years ago

Shane-XB-Qian commented 4 years ago

for example: cursor on line#2 e.g 'f' of 'formid', then :SidewaysRight, returned items is [], err reported out of index.

<form
id="formid"
name="myform"
method='post'
onsubmit="return checkUser();">

// looks it failed if start item had no 'tab' or 'space' indent -and- cursor on that item ..

AndrewRadev commented 4 years ago

That's a good catch. It seems I wasn't handling the scenario where the start pattern was ending just at the end of the line with the first argument immediately starting afterwards. I was finding the first arg with a "current column + 1" calculation, but I needed to overflow to the next line.

I've pushed a fix. Could you let me know if it works correctly for you as well?

Shane-XB-Qian commented 4 years ago

no, issue still there:

i: [{'end': '\s*/\?>', 'brackets': ['"''', '"'''], 'start': '<\k\+\_s\+', 'delimited_by_whitespace': 1}, {'end': ')', 'brackets': ['([{''"', ')]}''"'], 'start': '(\_s*', 'delimiter': ',\_s*'}, {'end': '\]', 'brackets': ['(
[{''"', ')]}''"'], 'start': '\[\_s*', 'delimiter': ',\_s*'}, {'end': '}', 'brackets': ['([{''"', ')]}''"'], 'start': '{\_s*', 'delimiter': ',\_s*'}]
v: [[21, 6, {'end': '\s*/\?>', 'brackets': ['"''', '"'''], 'start': '<\k\+\_s\+', 'delimited_by_whitespace': 1}], [12, 9, {'end': ')', 'brackets': ['([{''"', ')]}''"'], 'start': '(\_s*', 'delimiter': ',\_s*'}], [15, 3, {'e
nd': '}', 'brackets': ['([{''"', ')]}''"'], 'start': '{\_s*', 'delimiter': ',\_s*'}]]
i: [{'end': '\s*/\?>', 'brackets': ['"''', '"'''], 'start': '<\k\+\_s\+', 'delimited_by_whitespace': 1}]
v: []

i: is the definitions just enter it. v: is the valid_definitions after called the function you just modified. // the first i: and 'v:is fromsideways#Parse()`.

AndrewRadev commented 4 years ago

Sorry, my mistake -- I messed up the code in the push. Could you try again?

Shane-XB-Qian commented 4 years ago

yes, the original issue look fixed. but looks still a small one ....

<form
    id="formid"
name="myform"
method='post'
onsubmit="return checkUser();" >

if cursor at col('.') == 1 that position of line#2 id="formid" (not the 'i' of 'id' -vs- but at that 'tab' indent), though no err, but ':SidewaysRight' and/or ':SidewaysLeft' did not work if this case .. :)

Shane-XB-Qian commented 4 years ago

but looks this was not caused by this fix,

e.g:

<input type="text" value="" class="text2" name="username" id="userid"/>

if cursor on input type this middle space here, the heading space/whitechar of 1st item, this also can not ':SidewaysRight' and/or ':SidewaysLeft' .

so not sure if you thought it was an issue, or designed to like that ...

AndrewRadev commented 4 years ago

Right, I see what you mean.

so not sure if you thought it was an issue, or designed to like that ...

It's a consequence of how the plugin was designed, yes. Basically, the plugin looks for the beginning of a list in order to know what kind of items this list will have. For example, for [1, 2, 3], if the cursor is on "1", the plugin looks backwards for the wrapping list beginning, which is "[". But if the cursor is on the "[", the plugin will keep looking backwards, because it assumes your cursor is on an item. That way, you can move the inner list in [1, 2, [3 ,4]]. Nesting is not a problem for HTML, but the core logic is generic.

For HTML attributes, the beginning of the list is <input, including the whitespace after the tag. Otherwise, the first item would not be type="text", it would be type="text" with a space, and moving it around would lead to, for example <inputvalue="".

I can't really think of a good way to avoid this. The logic that moves the arguments is separate from the logic that calculates them, essentially the parsing produces a list of arg coordinates, like [{start: 1, end: 3}, {start: 5 , end: 7}] or something (except there's both line and column info, but you get the idea). So, once I have this list, I don't know if before the first argument there's whitespace or the start of the list, which could also be within another list.

Maybe there's some special handling I can do for the whitespace before the first argument, but I can't think of how right now, and it would definitely add some complexity. I would rather ask users to put the cursor on the argument they'd be moving or operating on, to reduce ambiguity. The plugin does work with the cursor between items, but I would consider that a minor convenience.

Shane-XB-Qian commented 4 years ago

ok, you are right, looks a little too aggressive if to support heading whitespace to be thought as 1st item, though so far i just felt it like that so .. anyway, there is no err (not quite like this issue as the original case), but just not work (not work like that way) ..

i am closing this ticket, thanks for your fix and explain, i like your plugin, btw. :)

AndrewRadev commented 4 years ago

i am closing this ticket, thanks for your fix and explain, i like your plugin, btw. :)

Thank you :). I'm glad you like it, and I appreciate you taking the time to report the issue.