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

Revert "fix: vim.eval('v:true') should return python bool #506" #562

Merged
merged 3 commits into from
Mar 27, 2024
Merged

Revert "fix: vim.eval('v:true') should return python bool #506" #562

merged 3 commits into from
Mar 27, 2024

Conversation

samuelqp
Copy link
Contributor

This reverts commit d549371. This fixes #523 and make UltiSnips usable. As mentioned by @jsr-p, the extra check is redundant and is not worth the slower performance.

This reverts commit d549371. This
fixes #523 and make UltiSnips usable. As mentioned by @jsr-p, the extra check
is redundant and is not worth the slower performance.
@samuelqp samuelqp marked this pull request as draft March 16, 2024 00:27
@samuelqp samuelqp marked this pull request as ready for review March 16, 2024 01:52
@samuelqp
Copy link
Contributor Author

The checks are failing even without the changes that I made (when the fork is identical to the current upstream) (https://github.com/samuelqp/pynvim/actions/runs/8304465825).

@justinmk
Copy link
Member

Thanks for the PR!

The tests are broken because they refer to a (internal) function that was removed in neovim/neovim@eb5d15e
Fixed in #563 , would you mind rebasing ?

@samuelqp
Copy link
Contributor Author

Thanks! Seems good now.

@samuelqp samuelqp deleted the branch neovim:master March 17, 2024 02:55
@samuelqp samuelqp closed this Mar 17, 2024
@samuelqp samuelqp deleted the master branch March 17, 2024 02:55
@samuelqp samuelqp restored the master branch March 17, 2024 02:55
@samuelqp samuelqp reopened this Mar 17, 2024
@justinmk
Copy link
Member

One more ask: can you add a "Known issues" section after this section which explains this quirk? So that we don't get repeat reports of #506

@samuelqp
Copy link
Contributor Author

One more ask: can you add a "Known issues" section after this section which explains this quirk? So that we don't get repeat reports of #506

Done

@justinmk justinmk merged commit 9f3e010 into neovim:master Mar 27, 2024
19 checks passed
assert h.legacy_vim.eval('1') == '1'
assert h.legacy_vim.eval('v:null') is None
assert h.legacy_vim.eval('v:true') is True
assert h.legacy_vim.eval('v:false') is False
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these tests still useful? Could change the expected values.

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.

num_to_str boolean check makes Ultisnips snippet engine laggy
2 participants