Skip to content

Robustness fixes and cleanups across prompt functions#14

Open
scorphus wants to merge 18 commits into
oh-my-fish:masterfrom
scorphus:improvements
Open

Robustness fixes and cleanups across prompt functions#14
scorphus wants to merge 18 commits into
oh-my-fish:masterfrom
scorphus:improvements

Conversation

@scorphus

Copy link
Copy Markdown
Member

Summary

Version detection robustness

  • Ruby falls back to ruby --version when rbenv reports no version
  • Python prompt works on systems where only python3 exists (e.g. recent macOS)
  • Rust version refreshes after any rustup subcommand, not only rustup default

Git info

  • Detached HEAD shows short SHA (prefixed :) instead of the git block disappearing
  • _git_ahead_count uses rev-list --count instead of log | grep '^commit' | wc -l | tr -d ' '
  • _git_dirty_remotes filter via fish's contains builtin instead of an external grep

Cleanup

  • _prompt_pwd takes its color as a parameter (palette-consistent with the other _prompt_* functions)
  • _prompt_versions drops an unused append parameter and a redundant tr -d '\n'
  • fish_right_prompt uses printf '|' instead of echo '|' (no stray newline mid right-prompt)
  • Blank-line cleanup across function bodies
  • Plus three commits I'd been carrying on my fork (virtualenv in title, special-char tweak)

Test plan

  • no-venv shell on macOS (python3-only) shows the system Python version
  • activating/deactivating a venv updates the prompt correctly
  • `git checkout ` keeps the git block visible (shows `:abcdef0`)
  • no remaining `grep`/`wc`/`tr` calls in prompt rendering

Pablo Santiago Blum de Aguiar and others added 18 commits January 25, 2022 22:23
If rbenv is installed but no Ruby version is configured for the current
directory, rbenv version-name returns empty and the prompt would show no
version at all. Clear ruby_version at the start of each invocation and,
after the rbenv branch, fall back to ruby --version whenever ruby_version
is still empty (in addition to the existing path-changed check).
On systems without a python2-era `python` binary (e.g. macOS where only
`python3` is shipped), _prompt_virtualenv returned early outside of a
virtualenv and the Python version was missing from the prompt entirely.
Inside a venv it worked because venvs provide a `python` symlink.

Probe for `python` first, then `python3`, and use whichever exists for
both the existence check and the --version call.
Previously the cached RUST_VERSION was only invalidated when the last
history entry contained `rustup default`. Other rustup invocations that
also change the active rustc — `rustup update`, `rustup toolchain
install`, `rustup override` — left a stale version in the prompt.

Broaden the match to any `rustup` subcommand, and use `string match`
instead of `echo | grep` to drop the per-prompt subshell + fork.
`git symbolic-ref HEAD` returns nothing when HEAD points at a tag or
arbitrary commit, so _git_branch_name produced an empty string and
_prompt_git returned early — the entire git block disappeared from the
prompt the moment you ran `git checkout <tag-or-sha>`.

Fall back to `git rev-parse --short HEAD` and prefix it with `:` so the
prompt still shows the current commit and the format makes the detached
state visually distinct from a regular branch name.
_git_ahead_count piped `git log` through `grep '^commit'` and `wc -l`
to count commits ahead of a remote — three forked processes per remote
per prompt rendering, plus a `tr -d ' '` to strip wc's padding.

`git rev-list --count <remote>/<branch>..HEAD` returns the same number
directly with a single process and no trailing whitespace, so the echo
wrapper and tr call drop out too.
The hardcoded origin|upstream filter is intentional — random remotes
(forks, mirrors, etc.) shouldn't pollute the prompt — but the previous
implementation shelled out to an external grep with a regex alternation
per prompt rendering.

Replace the pipe with fish's contains builtin and a continue guard.
Same filter, no fork, and the intent ("only these two remotes") is
spelled out as data rather than buried in a regex.
_prompt_pwd was the only _prompt_* function that hard-coded its color
(`set_color -o cyan`) instead of accepting it as an argument, so the
palette in fish_prompt was no longer the single source of truth — a
theme tweak meant editing two places.

Add a color parameter, declare bold cyan alongside the other palette
variables, and pass it in at both call sites.
`echo '|'` appended a newline mid right-prompt. Fish was getting away
with it visually, but a stray newline in the right-prompt buffer is
fragile — a future refactor or terminal quirk could surface it as a
broken layout.

Use `printf '|'` to emit just the character, matching how every other
glyph in this function is written.
The function signature declared a sixth parameter `append` that was
never referenced anywhere in the body. Dead since at least the move
into functions/. Drop it so the signature reflects what the function
actually uses.
The pipeline ended in `tr -d '\n'`, presumably to strip a trailing
newline. But `echo -n` (used at the start of the same pipeline)
doesn't emit one in the first place, so the tr invocation was a
no-op fork per prompt rendering.
The empty lines that separated logical groups of statements within
_git_dirty_remotes, _prompt_versions, fish_prompt, and fish_title were
inconsistent across the file (some functions had them, others didn't)
and added scroll height without aiding readability. Collapse them so
the bodies match the tighter style already used elsewhere.
@scorphus

Copy link
Copy Markdown
Member Author

@pedrosnk Pedrão! Can I be lucky and get you to review this PR?

Things worth flagging, in order of "could bite you if wrong":

  • _prompt_pwd now takes its color as a param. All other _prompt_* functions emit the escape sequence directly (via echo/printf), so the caller passes $cyan = (set_color -o cyan). I caught one bug mid-session where I wrote set_color $color (which expects a name, not the escape), worth a sanity check.
  • Version-detection caching changes (_prompt_rubies fallback, _prompt_virtualenv python3 probe, _prompt_rust string match): all three keep the existing caching idiom but broaden invalidation. Worth thinking about edge cases — e.g. does the python3 probe pick the right interpreter when both python and python3 exist (yes, but worth confirming)? Does the broader rustup * history match misfire on rustup completions or rustup which? Probably fine, but it's the kind of thing that's easy to overlook.
  • _git_dirty_remotes keeps the hardcoded origin|upstream filter on purpose — I almost dropped it, but the intent is to keep random remotes (forks, mirrors) from polluting the prompt. Now expressed via contains $remote origin upstream; or continue instead of | grep.
  • Detached-HEAD format is :<short-sha> (e.g. ‹:abc1234›). Taste call — happy to change if he hates the :.
  • The "drop blank lines inside function bodies" commit is mechanically large but pure whitespace; he can skim it.

History was rewritten a couple times (filter-branch passes for lowercase-after-colon and to clean backticks), so commit-by-commit review is fine but the bottom-line diff is what matters.

TIA :-*

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the scorphish Fish prompt’s robustness and reduces external-process usage by hardening language version detection, refining Git-state display (including detached HEAD), and adding an option to show command duration in the right prompt.

Changes:

  • Make Ruby/Python/Rust version detection more resilient (fallbacks, python3-only systems, rustup refresh triggers).
  • Improve Git prompt behavior/performance (detached HEAD short SHA, rev-list --count, fish contains filtering).
  • Cleanup/consistency updates across prompt functions and docs (config option, _prompt_pwd color parameter, printf vs echo).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
README.md Documents new right-prompt duration toggle option.
functions/fish_title.fish Enhances terminal title to include host and improved virtualenv naming.
functions/fish_right_prompt.fish Adds configurable duration vs time display and avoids newline issues.
functions/fish_prompt.fish Robustifies version detection, improves Git info display/perf, and aligns prompt layout with new right-prompt toggle.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread functions/fish_title.fish
function _prompt_rubies -a color -d 'Display current Ruby (rvm/rbenv)'
type -q ruby; or return
[ "$theme_display_ruby" = 'no' ]; and return
set -gx ruby_version
Comment on lines +39 to +41
set venv_name (basename "$VIRTUAL_ENV")
test $venv_name = ".venv"; and set venv_name (basename (dirname "$VIRTUAL_ENV"))
[ -n "$VIRTUAL_ENV" ]; and echo -n -s '@'$venv_name
type -q rustc; or return
[ "$theme_display_rust" != 'yes' ]; and return
if echo $history[1] | grep -q 'rustup default'; or not set -q RUST_VERSION
if string match -q 'rustup *' $history[1]; or not set -q RUST_VERSION
Comment on lines 116 to +118
set -l git_branch (_git_branch_name)
test -z $git_branch; and return
set git_branch (string replace -ar '(\.?[^/]{1})[^/]*/' '$1/' $git_branch)
Comment thread README.md
Comment on lines +44 to 49
* To display command duration in right prompt:

```fish
set -g theme_display_command_duration_in_right_prompt yes

```
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.

2 participants