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

Organization of statusline.lua #40

Open
anott03 opened this issue Feb 15, 2021 · 6 comments
Open

Organization of statusline.lua #40

anott03 opened this issue Feb 15, 2021 · 6 comments

Comments

@anott03
Copy link
Contributor

anott03 commented Feb 15, 2021

The Issue

Given that the changes made in #39, the example statusline component provided in statusline.lua is redundant since it in many ways is rewriting those functions (though granted it has a bit more logic).

My Proposal

What I propose is either a new module examples.lua, or simply adding the example to lsp-status.lua thus allowing it to take advantage of the individual component functions now provided.

Potential Objections

The logic included in the example component is such that a component is only rendered if the number of errors/warnings/messages is greater than 0. Since the component functions return strings not numbers, this logic cannot remain exactly as it was, but the effect can be recreated by checking if the third character of the component is '0'.

@wbthomason
Copy link
Collaborator

I'm not sure I agree with the proposal, primarily because a large number of users rely on the example statusline component out of the box (it was actually originally just an example in the README, but users requested it be added to the code), and I don't want to break their setups. I would be OK with rewriting the example component in terms of the new functions, to reduce code duplication, though - as you mention - this is slightly complicated by the new functions returning strings rather than numbers/tables. What do you think?

@anott03
Copy link
Contributor Author

anott03 commented Feb 16, 2021

I see your reasoning, though I would point out that if users are using this as intended, their config looks something like this:

local lspstatus = require('lsp-status')
...
lspstatus.status()

That should mean that assuming lsp-status.lua continues to return a function called status, we can refactor internally without concern.

@wbthomason
Copy link
Collaborator

Yep - that's more or less what I'm proposing. I'm just not in favor of removing the status function entirely.

@anott03
Copy link
Contributor Author

anott03 commented Feb 16, 2021

Oh yeah. Sorry I should have been clearer about that - I don't want to remove the function entirely, just potentially out of statusline.lua.

@anott03 anott03 closed this as completed Feb 16, 2021
@anott03 anott03 reopened this Feb 16, 2021
@anott03
Copy link
Contributor Author

anott03 commented Feb 16, 2021

(I didn't mean to close it)

@anott03
Copy link
Contributor Author

anott03 commented Feb 17, 2021

this is slightly complicated by the new functions returning strings rather than numbers/tables.

I've been thinking about this... maybe the new functions should return a table with a string and a number? I don't think it's a huge deal since with just a string it's a fairly simple matter of using string.sub but returning both values might be more convenient for this. However, that may make the UX a bit worse, since users would no longer be able to simply do require('lsp-status').status_X().

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

No branches or pull requests

2 participants