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(concealer): more precise anticonceal feature detection #1056

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

champignoom
Copy link
Contributor

The previous version comparison logic seems flawed: (0,10,0) should be greater than (0,7,1), not less.

To ensure that anticonceal is available, it's not enough to only check version being 0.10.0. We must also ensure that the subversion (the number after '-') is greater than 575 or something iirc.

@vhyrro
Copy link
Member

vhyrro commented Sep 6, 2023

I feel like this could be done much cleaner with the modern vim.version() and vim.version.parse() APIs, would be much more performant too :D
It allows for checking if it's a development (nightly) build alongside a specific revision where we could test for anticonceal specifically. Thoughts?

@champignoom
Copy link
Contributor Author

That would be great but I in the result of vim.version() I can't find the 1031 part of NVIM v0.10.0-dev-1031+gc431d820e.

@max397574
Copy link
Contributor

max397574 commented Sep 6, 2023

it has the __lt field in the metatable
I'll take a look at the source to see if we can use that

looks like this doesn't check the specific commits

@vhyrro
Copy link
Member

vhyrro commented Sep 8, 2023

That kinda sucks - given that we check for the current nvim version several times in the source code altogether I would prefer we do not parse strings on every iteration. Neorg is slow enough as it is 😆

Any alternate ideas perhaps?

@champignoom
Copy link
Contributor Author

I think It will only be parsed once at startup.

@vhyrro
Copy link
Member

vhyrro commented Sep 15, 2023

Alright, that's not too bad then. I'll attach a TODO for the future to move this to vim.version() when 0.10 becomes the next stable

@vhyrro vhyrro merged commit b0117a4 into nvim-neorg:main Sep 15, 2023
1 check passed
benlubas pushed a commit to benlubas/neorg that referenced this pull request Jan 11, 2024
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