Detect visual blocks too#22
Conversation
2d2de43 to
a3d15aa
Compare
mgmonteleone
left a comment
There was a problem hiding this comment.
Code Review Agent🛡️ with @mgmonteleone's authorization
Verdict: REQUEST CHANGES
Thanks for tackling visual-block selection support. The direction is useful, but I see two correctness issues that need to be fixed before merge.
Blocker: Ex-command path still cannot reliably detect visual block mode
The command handler and selection helper use mode() to infer whether the current selection is character, line, or block mode. That works only while Vim is still actively in Visual mode. When :Augment chat is invoked through the Ex-command path from a visual selection, Vim has already returned to Normal mode, so mode() reports n and the helper treats the selection as a normal multi-line span.
That means visual-block selections can still be expanded incorrectly when the command is invoked through the common :'<,'>Augment chat path. Please use visualmode() for the previous visual mode, or pass the resolved selection mode into augment#chat#GetSelectedText() / s:GetBufSelection() so Ex-command selections can preserve block semantics.
Blocker: removing the column adjustment introduces an off-by-one regression
The PR removes the existing col_start += 1 and col_end += 1 normalization for the . / v visual positions. That adjustment was compensating for the difference between the active visual cursor marks and the '< / '> marks used by the non-active path. Removing it changes character-wise selection bounds and can drop or include the wrong character at either edge.
Please restore the column normalization or replace it with a mode-aware normalization that is covered for character, line, and block selections. I would also strongly recommend adding manual or automated coverage for selecting a middle substring, selecting through the last character, and rectangular block selection through both direct visual invocation and the Ex-command range path.
mgmonteleone
left a comment
There was a problem hiding this comment.
Found a bug on the Ex-command path (:'<,'>Augment chat): mode() doesn't persist visual mode after leaving it, so the block detection fails. Use visualmode() ==# "\<C-v>" instead. Also, please restore the col_start += 1 for character visual mode, as it was only incorrect for block mode.
Detect block selections too, not only character and line selections.