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

feat(api): add vscode.get_status_item #1576

Merged
merged 1 commit into from
Oct 29, 2023

Conversation

xiyaowong
Copy link
Collaborator

  • Add internal action refresh-status-items
  • Delete StatusLineController
  • Change status bar priority from -1 to -10
  • Replace \n with space

vscode.get_status_item(id)

  • id (string): The identifier of the item
local test = vscode.get_status_item('test')
test.text = 'hello' -- Show the text
test.text = '' -- Hide the item
test.text = nil -- Close the item
test.text = '' -- error: The status item "test" has been closed

Why not directly support &statusline?

  1. Manual synchronization required

Firstly, changes to the statusline option need to be detected, and then nvim_eval_statusline is used to obtain the content. This can be a bit tedious.

  1. Performance issues

If the statusline is used, debouncing is necessary to avoid unnecessary refreshes. However, debouncing also causes delays when content is actually refreshed.

  1. No meaning

This API is intended to be used by plugin developers who want to be compatible with vscode-neovim, such as the prompt in flash.nvim. If statusline is used, the plugin needs to dynamically modify statusline, which obviously complicates simple things.

flash.nvim demo

status

@xiyaowong xiyaowong merged commit c20ce2f into vscode-neovim:master Oct 29, 2023
8 checks passed
@xiyaowong xiyaowong deleted the api/get_status_item branch October 29, 2023 21:38
status = vim.tbl_filter(function(i)
return i ~= ""
end, status)
M.action("refresh-status-items", { args = { status } })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doing rpc work on a field assignment is rather surprising. I would suggest avoiding that kind of OOP cleverness. It doesn't really gain anything and adds uncertainty to the entire API. And it defeats the main benefit of having a table representation of the data: it's no longer data, it's a "live object"....

Instead I would suggest something like a set_status_... function.

@@ -510,6 +511,20 @@ You can set `vscode.notify` as your default notify functions.
vim.notify = vscode.notify
```

##### `vscode.get_status_item(id)`

Creates a status item
Copy link
Collaborator

@justinmk justinmk Nov 13, 2023

Choose a reason for hiding this comment

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

I have no idea what this does. Why is it needed? Why can't users use :echo or something like that?

This looks like it might be an inner platform. We should avoid creating a third "plugin framework". Users already have 2 plugin frameworks: vscode and Nvim.

@justinmk
Copy link
Collaborator

This API is intended to be used by plugin developers who want to be compatible with vscode-neovim

That seems like we're getting into rather "nice to have" territory, which can be a source of maintenance burden for little gain.

@xiyaowong
Copy link
Collaborator Author

xiyaowong commented Nov 13, 2023

I have tried to use "echo", but it doesn't works well.

  1. What happens when multiple plugins use "echo" at the same time?
  2. Using echo '' clears all the messages.
  3. "Echo"updates the output.
  4. How to echo in cmdline mode

This PR just provides an API for manipulating the statusBar in VSCode.

@justinmk
Copy link
Collaborator

justinmk commented Nov 13, 2023

I have tried to use "echo", but it doesn't works well.

That is what all Nvim plugins use. If it doesn't work well, that's an Nvim bug (or a bug in the UI protocol).

This PR just provides an API for manipulating the statusBar in VSCode.

I'm strongly not in favor of that direction. Either we support Nvim plugins or we don't. Introducing a quasi-framework,

  1. is a maintenance burden
  2. distracts both plugin authors and vscode-neovim contributors from the actual goal of support Nvim plugins in vscode
  3. contradicts the tenets in CONTRIBUTING.md

@xiyaowong
Copy link
Collaborator Author

That is what all Nvim plugins use. If it doesn't work well, that's an Nvim bug (or a bug in the UI protocol).

But most nvim plugins use temporary statusline or floating window to display prompt information.

@xiyaowong
Copy link
Collaborator Author

  1. is a maintenance burden

The implementation is very simple and the degree of coupling is also very low.

  1. distracts both plugin authors and vscode-neovim contributors from the actual goal of support Nvim plugins in vscode
  2. contradicts the tenets in CONTRIBUTING.md

Anyway, many PRs are based on inconveniences I usually encounter. So the demand is there.

@justinmk
Copy link
Collaborator

justinmk commented Nov 13, 2023

But most nvim plugins use temporary statusline or floating window to display prompt information.

? That sounds like an assumption. And it's not really an argument for introducing yet another interface.

The implementation is very simple and the degree of coupling is also very low.

... Anyway, many PRs are based on inconveniences I usually encounter. So the demand is there

All tech debt accumulates with that same local analysis, it ignores the long-term cost.

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.

None yet

3 participants