You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(path): plan and write reviewed PATH edits with backups - #155
The neet-core side of reviewed PATH edits, as agreed in the issue. The Settings diff review comes in a follow-up PR.
What it does
New path::edit module. plan takes a Listing and the requested Remove(i) / Move(i, First | Last) changes and returns each shell file's text before and after. It is pure: file text comes in through a closure.
Origin::Line: the folder is taken out of its line in place, and the other folders stay. If only $PATH is left, the whole line goes. If nothing would be left, the change is refused.
Only lines neet can fully read are changed: PATH= or export PATH=, with an optional double-quoted value and comment, holding only full paths, $HOME/${HOME}/~ (~ only without quotes), and $PATH/${PATH}. neet refuses $(...), backticks, &&, ;, single quotes, other variables, path=(...), continued lines, and lines inside an if, a loop, a function, or brackets.
Moves take the folder out of its line and add export PATH="<folder>:$PATH" (or "$PATH:<folder>") at the end of the # >>> neet >>> block in ~/.zshrc, through a new zsh::append shared with Configure. Moves from bash or fish files are refused, because only zsh reads .zshrc.
Origin::Tool, Origin::System, and Origin::Unknown are refused, so mise, brew shellenv, and /etc/paths are never changed.
Every copy of a folder points at the first line that names it. A removal that would also take away a copy you keep is refused, unless another line still adds that folder.
prepare opens the files with dotfiles::change::Edit::begin, so view-only files and chezmoi work as they do for Dotfiles. write runs change::check on each new text first and writes nothing if a check fails. Then it writes each file through Edit::finish, with its backup in ~/.local/state/neet/backups/dotfiles, so Dotfiles can restore it.
cargo test --workspace --all-features: run on Linux in Docker as a non-root user with zsh installed. 9 new tests pass. The only failures are the same 12 macOS-only tests (osascript, defaults, launchctl, getconf) that also fail on master there.
cargo check --workspace --all-targets --all-features --locked with Rust 1.88
The tests cover: lines with several folders, removing a line left with only $PATH, refusing $(...), backticks, conditionals, ;, other variables, quotes, arrays, continued lines and if/function blocks, refusing tools, system lists and unknown origins, moves into a new or existing neet block, a broken block, moves from bash files, repeated copies, lines that changed after the listing, backups and restore through Dotfiles, and writing nothing after a failed zsh -n or a file changed after review.
Notes
The diff is larger than the 200-line commit guideline (about 440 lines of code and 320 of tests). I can split it into two commits, the planner and then prepare/write, if you prefer.
docs/ARCHITECTURE.md: one table row now lists path/. It needs your approval. I left SAFETY.md and FEATURES.md alone, because Shell PATH stays view-only until the Settings PR.
Question: moving a folder that was added in .zshenv or .zprofile into the .zshrc block means only interactive shells get it. Should those moves be refused, or shown as a warning in the review?
Thanks a lot for this, it's really well done. The planner follows exactly what was agreed in #148, the "when in doubt, refuse" approach is just right for editing people's shell files, and going through Edit::begin/finish means backups and restore work for free. The tests are great too.
Regarding your questions:
One commit or two: one is totally fine here, no need to split.
ARCHITECTURE.md: the path/ row is approved, thanks for leaving SAFETY/FEATURES for the Settings PR.
Moves from .zshenv / .zprofile: let's refuse those for now, the same as bash/fish files. .zshenv is also read by scripts and editors, so moving a folder into .zshrc could quietly break them. We can add a warning later if people ask for it. This can go in the Settings PR, no need to change this one.
plan is described as pure, but source() calls shell_lines(home), which reads from disk. It could go through the read closure instead.
Moving two copies of the same folder writes it twice in the block ("/a:/a:$PATH").
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
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 #148
The
neet-coreside of reviewed PATH edits, as agreed in the issue. The Settings diff review comes in a follow-up PR.What it does
path::editmodule.plantakes aListingand the requestedRemove(i)/Move(i, First | Last)changes and returns each shell file's text before and after. It is pure: file text comes in through a closure.Origin::Line: the folder is taken out of its line in place, and the other folders stay. If only$PATHis left, the whole line goes. If nothing would be left, the change is refused.PATH=orexport PATH=, with an optional double-quoted value and comment, holding only full paths,$HOME/${HOME}/~(~only without quotes), and$PATH/${PATH}. neet refuses$(...), backticks,&&,;, single quotes, other variables,path=(...), continued lines, and lines inside anif, a loop, a function, or brackets.export PATH="<folder>:$PATH"(or"$PATH:<folder>") at the end of the# >>> neet >>>block in~/.zshrc, through a newzsh::appendshared with Configure. Moves from bash or fish files are refused, because only zsh reads.zshrc.Origin::Tool,Origin::System, andOrigin::Unknownare refused, so mise,brew shellenv, and/etc/pathsare never changed.prepareopens the files withdotfiles::change::Edit::begin, so view-only files and chezmoi work as they do for Dotfiles.writerunschange::checkon each new text first and writes nothing if a check fails. Then it writes each file throughEdit::finish, with its backup in~/.local/state/neet/backups/dotfiles, so Dotfiles can restore it.Checks
cargo fmt --all --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-features: run on Linux in Docker as a non-root user with zsh installed. 9 new tests pass. The only failures are the same 12 macOS-only tests (osascript, defaults, launchctl, getconf) that also fail onmasterthere.cargo check --workspace --all-targets --all-features --lockedwith Rust 1.88$PATH, refusing$(...), backticks, conditionals,;, other variables, quotes, arrays, continued lines andif/function blocks, refusing tools, system lists and unknown origins, moves into a new or existing neet block, a broken block, moves from bash files, repeated copies, lines that changed after the listing, backups and restore through Dotfiles, and writing nothing after a failedzsh -nor a file changed after review.Notes
prepare/write, if you prefer.docs/ARCHITECTURE.md: one table row now listspath/. It needs your approval. I leftSAFETY.mdandFEATURES.mdalone, because Shell PATH stays view-only until the Settings PR..zshenvor.zprofileinto the.zshrcblock means only interactive shells get it. Should those moves be refused, or shown as a warning in the review?