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

Proposal: sign_setlist #4557

Closed
myitcv opened this issue Jun 18, 2019 · 18 comments
Closed

Proposal: sign_setlist #4557

myitcv opened this issue Jun 18, 2019 · 18 comments

Comments

@myitcv
Copy link

myitcv commented Jun 18, 2019

Is your feature request related something that is currently hard to do? Please describe.

Per https://groups.google.com/d/msg/vim_dev/aW2buOgZ6qI/fpxkp-V8AQAJ

In https://github.com/leitzler/govim (master branch) Pontus is adding support to govim for signs that correspond to quickfix entries.

This is done by calling sign_place/sign_unplace as required to get the signs for a buffer in the correct state.

If there are lots of errors in a file, then there can be a large number of calls to sign_place/sign_unplace made by govim in rapid succession.

Describe the solution you'd like

A function:

sign_setlist({bfnr}, {list} [, {action}])

that sets the signs for buffer {bufnr} to {list}. {action} is similar to setqflist in that the caller can specify whether to create, replace or add to existing signs.

Describe alternatives you've considered

The only apparent alternative at the moment is to call sign_getplaced and then make the necessary calls to sign_place/sign_unplace; over a channel this becomes expensive.

Additional context

n/a

@vim-ml
Copy link

vim-ml commented Jun 29, 2019 via email

@myitcv
Copy link
Author

myitcv commented Jun 29, 2019

Thinking about this some more, we should modify the existing sign_place() function to always accept a List argument

Just to say, I don't have any strong feelings either way. I'll defer to those with more experience of the Vim API and how changes are made.

@brammool
Copy link
Contributor

brammool commented Jun 29, 2019 via email

@vim-ml
Copy link

vim-ml commented Jun 29, 2019 via email

@vim-ml
Copy link

vim-ml commented Jul 1, 2019 via email

@vim-ml
Copy link

vim-ml commented Jul 1, 2019 via email

@vim-ml
Copy link

vim-ml commented Jul 5, 2019 via email

@myitcv
Copy link
Author

myitcv commented Jul 5, 2019

My only comment here would be that sign_setlist as proposed in my original description not only handles setting of signs but also unsetting. That is, it ensures the buffer's sign list reflects the argument. Because in the context of my example I have the list of errors for a buffer (the same list that populates the quickfix window) and I simply want to say "now ensure the signs for this buffer are this list X", just as I call setqflist to update the quickfix window.

So from a selfish perspective I'd prefer not to have to keep track of/derive the delta to be applied, I'd like to just call a function that does all of that for me.

@brammool
Copy link
Contributor

brammool commented Jul 5, 2019 via email

@vim-ml
Copy link

vim-ml commented Jul 5, 2019 via email

@myitcv
Copy link
Author

myitcv commented Jul 5, 2019

That's just one use case.

Yes, indeed: quite accept that I'm only speaking about my particular use case here.

The reason behind me being attracted to this approach is that it makes it simpler from a channel-based plugin to do everything with a single call (reduces round trips), which also means less flicker for the user. As I understand it, with two calls (clear then set) there is potential (likely?) for flickering.

@brammool
Copy link
Contributor

brammool commented Jul 5, 2019 via email

@myitcv
Copy link
Author

myitcv commented Jul 5, 2019

The function calls can be in one message, right?

Very true.

@vim-ml
Copy link

vim-ml commented Jul 9, 2019 via email

@vim-ml
Copy link

vim-ml commented Jul 9, 2019 via email

@vim-ml
Copy link

vim-ml commented Jul 10, 2019 via email

@myitcv
Copy link
Author

myitcv commented Jul 10, 2019

The function calls can be in one message, right?

Very true.

As a follow up to this point (and hopefully not straying too far off topic on this thread), we now have a first cut of batching calls in govim (which could easily be extended to include the other channel functions if required):

govim/govim@bcba0c8

(which was then accompanied by a later bug fix govim/govim@2848a08)

The approach works well.

That said, the changes in this will likely still be a benefit I'd assume? Because we move from multiple function calls to two calls with list arguments.

@vim-ml
Copy link

vim-ml commented Jul 14, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants