Visual selection and passed range conflated after 9d43

[Konfekt]

After the simplification commit 9d43ef6#diff-c5343a5d13850471f849776609ce57fc060df53e28763bb19f8c407e81854970R191 (in response to #112 (comment) ?) users could be surprised if the lines covered by the last visual selection coincide with those passed to it as it effectively conflates these two situations.
Is this what the truty comment refers to?

One workaround is outlined at artificial range parameter and I guess this is what g:vim_ai_is_selection_pending was introduced if I can remember somewhat correctly.

While removing it simplifies code and improves developer experience, user experience suffers in this sense.

[madox2]

Hi, thanks for the prompt review! I couldn't recall the issue with the last visual selection, therefore I tested all use cases I could think of and hope this is not an issue anymore. Could you remind me how to reproduce it in the plugin?

[Konfekt]

So in a buffer reading

Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

select the first sentence by vis<esc> and then run, say :.AIChat on the whole line.
It will still only treat the selected sentence

Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

[madox2]

In the commit above, there was an intended breaking change: plugin does not include current line into the prompt (unless it is visually selected). Is this still an issue then?

[Konfekt]

If the intent was to break single line ranges, then it was as intended, but from a user perspective something that works is more desirable.

Regarding multiline selections, have a buffer reading

A 1
B 2
C 3
D 4
E 5
F 6
G 7
H 8
J 9
K 10
L 11
M 12

and hit <c-v>G<esc>:%AIChat<cr>. Then one would expect the whole buffer, but the final 12missing.

In fact, this hints at another bug that <c-v> is not respected but replaced by line-wise selection v if one runs <c-v>G:AIChat<cr>; should this go into a separate issue?

[madox2]

The motivation is that hitting :AI without selection should just quickly answer questions, without user thinking what line is cursor on. If they want to include the line, they can hit c-v.

and hit G:%AIChat. Then one would expect the whole buffer, but the final 12missing.

I cannot reproduce this, it works as expected in my environment. Could you explain more or share a gif?

[Konfekt]

Since two bugs are at work here, maybe the following two-line example makes this clearer:

In a buffer reading

Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. 
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. 

hit $BvEE<esc>:%AIChat<cr> (here the next line should be jumped to, which depends on &whichwrap).

Then only

laborum. 
Lorem

is handled

[Konfekt]

Could you explain more or share a gif?

That's the first time I try recording a Gif, maybe https://imgur.com/mTyqhhG helps?

[madox2]

I see, now I remember this weird issue. What do you think of using histget(':', -1) to determine if it is visual selection, range or none? (visual selection should start with '<,'>, ranges with something else)

[Konfekt]

Sounds great as it seems to work; as it's been around since version 5 I wonder why this approach has not been used yet. Maybe nobody thought of it since Vim's vast

[madox2]

The funny thing is that I can reproduce all buggy scenarios above with the version before the commit, e.g. ea83fdc

With histget I identified one problem - it would probably not work with custom key mapping

[madox2]

In this 3122b84 I have re-enabled single line ranges. I still wonder what was the reason g:vim_ai_is_selection_pending was introduced, because it doesn't seem to fix issues above.

[Konfekt]

I have yet to check but g:vim_ai_is_selection_pending was supposed to reliably tell if the user entered : command-line mode from a visual selection, with the execption of the issue closed by the commit leading to this issue when the cursor didn't move after entering visual mode

[Konfekt]

You're right. These two tests also fail before the penultimate commit.

They work if the cursor moves in-between the two mode changes, though, similar to the issue that was closed.
So if you checkout HEAD~2 and then insert some movement between visual mode $BvEE<esc> and command-line mode :%AIChat<cr>, say

$BvEE<esc>h:%AIChat<cr>

instead of

$BvEE<esc>:%AIChat<cr>

then they pass.

Not moving at all between changing modes is rather rare, that's why it went undiscovered for a while