Conversation
The alias and subcommand handlers each duplicated: - the project-trust check (12+ inline copies) - the ActiveProfile-empty-session fallback ladder (6 inline copies) Both helpers (resolve_target, require_project_trust, get_profile_mut) already existed for the var handlers — applying them across the board collapses ~130 lines of repeated dispatch into the same three-arm match every handler now uses. Also aligns resolve_target's ActiveProfile resolution with the existing resolve_profile_mut behaviour (.last() — most recently activated profile, highest precedence in the engine merge). This fixes a latent inconsistency where am var defaulted to active_profiles[0] while am alias used .last().
…s in bin Six near-identical local-mutation helpers (add/remove × alias/subcommand/var) each reimplemented the same find-or-prompt-or-create dance. Two closure-taking helpers now own the shape; each helper collapses to ~5 lines. Side-effect fixes: - add_local_var was missing the "found existing .aliases at <parent>, add there?" prompt that alias/subcommand had — now consistent. - add_local_var had a dead-code identity branch (both arms returning local_path) — gone.
Two divergent paths used to mutate `.aliases`: - lib's execute_effect (used by am-tui and tests) → save_project_with → full ritual (write file + recompute hash + update security_config + save). - bin's execute_effects (used by the main binary) → standalone helpers → just write file. No hash, no security update. Result: `am add -l foo bar` left the security hash stale, so the next read saw the file as Tampered. The TUI doing the same operation didn't. Fix: extract refresh_project_trust_at on AppModel (the post-save bookkeeping half of save_project_with), and have the bin's interactive helpers call it after their own write. save_project_with now delegates to it too — single source of truth for "the file changed, trust must follow." The bin keeps its interactive parent-prompt logic (which the lib path can't support since it has no stdin/stdout access), but the security accounting is now consistent across both paths.
`am var set -l opts "-C opt-level=3"` rejected the value because clap parsed `-C` as an unknown flag. Set `allow_hyphen_values = true` on the value arg — values are opaque strings and shouldn't be parsed as flags. Adds a regression test pinning the parse behaviour.
The fish/bash/zsh/powershell am-wrappers re-source sync output after mutations like add/remove/profile-use, but vars were missing from the case lists. Result: changing a var didn't update aliases that referenced it until the next cd hook (or a manual `am sync`) — defeating the whole "set once and forget" UX of vars. Add `var v` (with subactions `set`/`unset`) to all four wrappers. The init snapshots regenerate accordingly.
Instead of `print!` inside the InitShell handler, return the rendered init script as `Effect::Print(output)`. Brings InitShell in line with GetVar/ListVars and the rest of the effects pattern, and is one step closer to a fully pure update().
Var substitution was reusing the quote-aware walker designed for
positional args (\`{{1}}\`/\`{{@}}\`), which breaks out of single-quoted
regions so shell variable references can expand. That's wrong for vars:
their values are literal text, not expansions.
For \`RUSTFLAGS='{{opts}}' cargo run\` with opts="-C opt-level=3", the
walker emitted \`RUSTFLAGS=''-C opt-level=3''\` — two empty single-quoted
strings sandwiching unquoted \`-C opt-level=3\`. Fish then parsed \`-C\`
as a command and failed.
Switch to a plain regex replacement. The user's quotes wrap the value
as written: \`'{{opts}}'\` becomes \`'-C opt-level=3'\`, which is a
single token. Same logic for double-quoted and unquoted forms.
Documented limitation: a value containing the same quote char the user
wrapped \`{{name}}\` with will produce broken shell — the user picks
the quoting, the value is opaque text.
Removes the now-unused \`substitute_quote_aware_for_vars\` shim too.
The --wraps target was computed via cmd.split_whitespace().next(), which
splits on spaces blindly. For an alias body like
\`RUSTFLAGS="-C opt-level=3" cargo run --release --bin am\` the first
"word" became \`RUSTFLAGS="-C\` — an unbalanced double quote — and ended
up in the emitted line as
complete -c r --wraps "RUSTFLAGS="-C"
Fish then accumulated unclosed-string state across every later line of
sync output, eventually erroring with "Expected a command, but found an
incomplete token" on whatever line tipped it past EOL.
Replace the raw split with first_command_word() that skips leading
\`KEY=value\` env-var prefixes (with quote-aware values) and refuses
candidates containing quote characters. The result for the offending
case is now \`complete -c r --wraps "cargo"\` — both correct and useful
(autocompletion follows the actual program). Aliases that genuinely
start with a quoted token simply skip the --wraps line rather than
emit broken fish.
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.
Closes #103.
What
Named variables that substitute into alias commands as
{{name}}. Set once witham var set, scoped local / profile / global. Variables resolve scope-locally — an alias only sees vars defined at its own scope.Why
Recurring command fragments (compiler flags, paths, version numbers) were duplicated across aliases. Vars factor them out without inventing a new alias-of-aliases pattern. Set once, edit one place, sync rebuilds everything.
Working forms of the example alias
The same alias body can be expressed several shell-quoting ways. All of these end up with the alias body
RUSTFLAGS="{{opts}}" cargo run --release --bin am(or the'-variant) and produce a working fish function afteram sync:A bare-unquoted
{{opts}}parses but breaks at runtime if the value contains whitespace — that's expected shell semantics, the user picks the quoting:Behavioural notes
'{{opts}}'substitutes to'-C opt-level=3'(one token), not''-C opt-level=3''(two empty strings + bare flag).am var set/unsettriggers a sync via the shell wrappers, so dependent aliases come alive immediately.am var set <name> <value>accepts hyphen-leading values like-C opt-level=3(clapallow_hyphen_values).Side-effect cleanups landed in this PR
add_local_aliasetc.) now go through a sharedupsert_local_aliases/mutate_existing_localpair — six near-clones collapse to ~5 lines each. Fixes a dead-code identity branch and an inconsistent missing parent-prompt.AppModel::refresh_project_trust_at, matching the lib path used byam-tuiand tests.am add -l ...no longer leaves the hash stale.add/remove/var) share oneTargetScopeArgsclap struct.update.rsnow go throughresolve_target+get_profile_mut— ~130 lines of repeated dispatch ladders gone.complete --wrapstarget skipsKEY=valueenv-var prefixes (with quote-aware values) — fixes the parse-error cascade whereRUSTFLAGS="-C opt-level=3" cargo runproducedcomplete -c r --wraps "RUSTFLAGS="-C".update::InitShellnow routes its output throughEffect::Printinstead of an inlineprint!, one step closer to a fully pureupdate().