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(PVS/V583): The '?:' operator always returns one and the same value #17790

Merged
merged 1 commit into from
Mar 20, 2022

Conversation

dundargoc
Copy link
Member

Not entirely sure if this is the correct fix, because the code itself
would make sense if STATUS_HEIGHT had another value. Is it to make it
possible to change the value of it in the future perhaps?

@dundargoc
Copy link
Member Author

@famiu Mind taking a look?

@famiu
Copy link
Member

famiu commented Mar 20, 2022

@famiu Mind taking a look?

As you guessed, it is in fact to make it possible to change the value of STATUS_HEIGHT in the future. Feel free to change that (and also the resize_frame_for_status function) if you feel like changing the value would never be necessary

@dundargoc
Copy link
Member Author

I don't mind it really, I'm just not sure if the most "correct" solution would be to remove the ternary operator (like how it is currently) or just add an ignore directive/comment on those lines. I'll ask an unrelated third party on their opinion.

Thanks for your input :)

@dundargoc dundargoc requested a review from gpanders March 20, 2022 22:06
@gpanders
Copy link
Member

My unrelated third-party opinion is that I agree with these changes. If and when the day comes that we want to change STATUS_HEIGHT it’s a trivial refactor, but for now this is a good simplification (YAGNI or whatever).

@gpanders gpanders merged commit e9b53f3 into neovim:master Mar 20, 2022
famiu added a commit to famiu/neovim that referenced this pull request Mar 21, 2022
Since neovim#17790 being merged means we can assume the value of `STATUS_HEIGHT` to always be 1, this commit removes code that's unnecessary if `STATUS_HEIGHT` is 1.
famiu added a commit to famiu/neovim that referenced this pull request Mar 21, 2022
Since neovim#17790 being merged means we can assume the value of `STATUS_HEIGHT` to always be 1, this commit removes code that's unnecessary if `STATUS_HEIGHT` is 1.
@dundargoc dundargoc deleted the pvs/v583 branch March 24, 2022 12:48
justinmk pushed a commit that referenced this pull request Mar 27, 2022
Since #17790 being merged means we can assume the value of `STATUS_HEIGHT` to always be 1, this commit removes code that's unnecessary if `STATUS_HEIGHT` is 1.
dmitmel pushed a commit to dmitmel/neovim that referenced this pull request Apr 16, 2022
Since neovim#17790 being merged means we can assume the value of `STATUS_HEIGHT` to always be 1, this commit removes code that's unnecessary if `STATUS_HEIGHT` is 1.
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.

3 participants