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

Fix for adding flexible components #80

Merged
merged 3 commits into from
Oct 24, 2022
Merged

Conversation

MenBrial
Copy link

Use vim.tbl_contains to ensure a value is not already in the
priorties list. priorities[priority] may return erroneous answers.
For example, if the first component has a priority value of 2 and the
second component has a priority value of 1, priorities[1] returns
the priority of the first component (2) and not nil as expected. This
results in ignoring the second component.

Use `vim.tbl_contains` to ensure a value is not already in the
`priorties` list. `priorities[priority]` may return erroneous answers.
For example, if the first component has a priority value of `2` and the
second component has a priority value of `1`, `priorities[1]` returns
the priority of the first component (2) and not `nil` as expected. This
results in ignoring the second component.
@rebelot
Copy link
Owner

rebelot commented Oct 23, 2022

Good call, thanks. Also using a set like could solve it.

priorities[priority] = true and then use vim.tbl_keys to retrieve priorities. What approach do you think is more efficient?

rebelot added a commit that referenced this pull request Oct 23, 2022
…ents, correct priorities and improve sorting (#80).
@rebelot rebelot merged commit d26197e into rebelot:master Oct 24, 2022
@MenBrial
Copy link
Author

I would think that priorities[priority] = true is faster: it uses only one internal call while my solution uses a call to vim.tbl_contains, which is a Neovim function implemented in Lua, and possibly an internal call in addition.

Therefore it is probably better to use priorities[priority] = true even if the impact is probably limited as I except there would only be a limited number of flexible components.

@MenBrial MenBrial deleted the flexible_fix branch October 30, 2022 14:12
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