Repository navigation
feat(config): per-platform shell and keyboard layout - #1856
Merged
Merged
Conversation
One config file shared across machines could not say which shell each of them runs, and Windows got its shell from a per-build override plus a text rewrite in config init. kb_layout_to_use had the same problem, since its values are platform-specific names. general.shell is a new per-platform command line for exec steps: [general.shell] darwin = ["/bin/zsh", "-lc"] linux = ["/bin/bash", "-lc"] windows = ['C:\Windows\System32\cmd.exe', "/c"] kb_layout_to_use now takes the same table. Both, and primary_modifier, share one PerPlatform type. Each takes a table keyed by darwin, linux and windows, or one value for all three, so every existing kb_layout_to_use line loads unchanged. A table only changes the platforms it names, and a key that names no platform refuses the file. exec_shell and exec_shell_args are deprecated and are removed in v2. Until then each one that is set replaces its part of general.shell on every platform, and loading warns about it. The Windows default is now the literal C:\Windows\System32\cmd.exe rather than %SystemRoot%, so an install on another drive sets the windows entry. The explicit-defaults guardrail now reads generic literal types, which it skipped before.
Contributor
[Medium impact] The PR appears safe to merge; all three previous findings are fixed and no new actionable issues remain. SummaryThis PR adds per-platform shell commands and keyboard layouts while keeping the deprecated shell settings available until v2.
Reviews (2) · Last reviewed commit: "fix(config): accept one shell value and ..." · Reviewed by Greptile |
One value for general.shell could never load. It applies to every platform, and the windows entry was checked as a Windows path on every machine, so no single path passed. Only the entry for the running platform now needs an absolute path. Every entry still needs an argument. neru config set general.kb_layout_to_use <id> stopped working once the option became a table. Naming a per-platform option without a platform now sets every platform, as the single-value form in a file does. The loader tests use a path that is absolute on the host, so they pass on Windows.
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR makes the shell that
execsteps run, andgeneral.kb_layout_to_use, configurable per platform, using the same table shapegeneral.primary_modifierintroduced in #1854.What changed for users.
general.shellis new. It is the full command line (shell plus arguments) anexecstep runs, with the step's command string appended last. Every entry must start with a path that is absolute on its own platform and have at least one argument. Neru checks all three entries on every machine.general.kb_layout_to_usetakes the same per-platform table.shell,kb_layout_to_use,primary_modifier) accept a table keyed bydarwin,linuxandwindows, or one value for all three. A table only changes the platforms it names. A misspelled platform key such asmacosrefuses the file with"macos" is not a platform. Use darwin, linux or windows.neru config set general.shell.linux '["/bin/zsh", "-c"]'andneru config set general.kb_layout_to_use.darwin <id>change one entry at runtime.Existing configs keep working.
kb_layout_to_use = "..."line loads as one value for every platform, with no warning.exec_shellandexec_shell_argsare deprecated and are removed in v2. Until then each one that is set replaces its part ofgeneral.shellon every platform, so a config written for one machine runs the same shell as before. Loading andneru config validatewarngeneral.exec_shell is deprecated and is removed in v2. Move it to general.shell(same forexec_shell_args).neru config setdoes not print it, as with every config warning today.C:\Windows\System32\cmd.exeinstead of%SystemRoot%\System32\cmd.exe, because a table cannot read environment variables. A Windows install on another drive sets thewindowsentry.Related Issues
Follows #1854.
Target Platform
Type of Change
feat— New featurefix— Bug fixrefactor— Code restructuring (no behavior change)perf— Performance improvementdocs— Documentation onlytest— Adding or updating testschore— Build, CI, dependencies, toolingCross-Platform Checklist
//go:build darwin)CodeNotSupported)The only platform-specific change is
config_windows.go, whose shell override is removed. It is type-checked bycheck-crossandlint-cross. The Windows default is not run locally.General Checklist
just fmt)just cipasses — the same checks CI runs, on your host only (formatcheck, lint, vet, build, a CGO-off type-check of the Linux and Windows
builds, tests including a unit
-racepass, vulnerability scan); CIruns them on macOS, Linux and Windows
subject written for users — this PR squash-merges, so the title is what
Release Please ships in the changelog, not the commits on the branch
Additional Context
N/A