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

Allow :LSClientFindActions with visual selection #179

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

natebosch
Copy link
Owner

Towards #178

Uses a hack to try to figure out if the user is likely to have set
reasonable marks for the range and is running from a visual selection so
that lines were passed with :'<,'>LSClientFindCodeActions.

Towards #178

Uses a hack to try to figure out if the user is likely to have set
reasonable marks for the range and is running from a visual selection so
that lines were passed with `:'<,'>LSClientFindCodeActions`.
@@ -5,7 +5,8 @@ function! lsc#edit#findCodeActions(...) abort
let ActionFilter = function("<SID>ActionMenu")
endif
call lsc#file#flushChanges()
let params = lsc#params#documentRange()
let l:usingRange = a:0 > 2 && (a:2 != 1 || a:3 != line('$'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to check a:2 != 1 || a:3 != line('$')?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my hack to know whether we were called with a range. If we're called like :LSClientFindCodeActions then the -range=% at the command definition will fill in 1,$ and we're identifying that case here. If we're called like :'<,'>LSClientFindCodeActions then it shouldn't match 1,$ and we'd know to actually look at the visual selection marks.

It does mean you can't actually do :1,$LSClientFindCodeActions and pass the entire doc in the range to the server...

Copy link

@slonoed slonoed Apr 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be && instead of || then?

Also sending the whole document range can be useful. Example: wrapping code in IIFE.
Maybe it is better to expose function with argument isRange and use it in keybindings instead of command which tries to guess visual mode?
I use this one

vnoremap <silent> ga :call lsc#edit#findCodeActions(lsc#edit#filterActions(), 0, 0)<CR>

@slonoed
Copy link

slonoed commented May 13, 2019

Hi! Are you going to merge this? Or you looking for another solution? Can I help you somehow?

@natebosch
Copy link
Owner Author

Sorry for the slow reply - I'd like to play with this a bit more before merging. I'd prefer to avoid a situation where it's necessary to map directly to lsc#edit#findCodeActions...

Is a keymap your preferred way to use this? We could also explore support for visual mode mappings in the config and a way to pass a flag to tell it explicitly that it's using visual mode.

@slonoed
Copy link

slonoed commented May 23, 2019

@natebosch don't worry. I understand that this is an open source project and you work on it when you have time 👍

Yes, keymap is a preferred way. I'm using actions quite a lot and it doesn't make sense to type command every time.

As a user I would like to use the same keybingind (ga) for both normal and visual modes and consistent behaviour (open list of code actions). This is more natural way for me.
In the same time it is fine for me to use a different keybinding. For me most important to have ability to use ranges in code actions and not a way to do it.
Also, as a user I don't have any preferred way to crate keybinding.

I hope it helps.

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

Successfully merging this pull request may close these issues.

2 participants