Skip to content

fix(#357): comp-time Win. ext. for run handler#358

Merged
MordechaiHadad merged 2 commits into
MordechaiHadad:masterfrom
MrDwarf7:fix/run_handler-windows-ext
Nov 27, 2025
Merged

fix(#357): comp-time Win. ext. for run handler#358
MordechaiHadad merged 2 commits into
MordechaiHadad:masterfrom
MrDwarf7:fix/run_handler-windows-ext

Conversation

@MrDwarf7

Copy link
Copy Markdown
Contributor

This addresses issue #357 by adding comp-time handling for Windows systems.
Previously the run_handler fn was not not correctly appending the .exe extension to the Neovim binary path, this hot-fix addresses this.

Closes #357

@MordechaiHadad MordechaiHadad left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you change this to a simple let bin_path = if cfg! clause instead of like this, the current approach is good but i prefer it to be concise.

@MrDwarf7

Copy link
Copy Markdown
Contributor Author

@MordechaiHadad - Yeah, happy to update this to use a conditional macro. Wrote it this way as conditional macros (if cfg!(...) {...} ) stay in the binary, were as this way they don't get compiled if it's not the resulting platform (is why I used not(windows) as opposed to just unix/linux for the deco).

This addresses issue MordechaiHadad#357 by adding comp-time 
handling for Windows systems. 
Previously the `run_handler` fn was not not 
correctly appending the `.exe` extension 
to the Neovim binary path, this 
hot-fix addresses this.

Closes MordechaiHadad#357
PR Review comment requested to use
`let bin_path = if cfg!(...) {...}` over platform annotations/decos 
for binary path. This update addresses that.
@MrDwarf7 MrDwarf7 force-pushed the fix/run_handler-windows-ext branch from 670a5eb to edcd230 Compare November 27, 2025 02:05
@MrDwarf7

Copy link
Copy Markdown
Contributor Author

This has been updated to match master and the changes you request have been added as well.

@MordechaiHadad

Copy link
Copy Markdown
Owner

@MordechaiHadad - Yeah, happy to update this to use a conditional macro. Wrote it this way as conditional macros (if cfg!(...) {...} ) stay in the binary, were as this way they don't get compiled if it's not the resulting platform (is why I used not(windows) as opposed to just unix/linux for the deco).

Fair I don't care that much for having those compiled its very small imo to early optimize them.

@MordechaiHadad MordechaiHadad merged commit 5f5fb53 into MordechaiHadad:master Nov 27, 2025
21 checks passed
@MrDwarf7 MrDwarf7 deleted the fix/run_handler-windows-ext branch November 27, 2025 10:57
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.

bob run <version> missing .exe on Windows

2 participants