[F/+] Fix Python fallback crashes and greatly improve shell completion#519
Open
Flaykky wants to merge 2 commits into
Open
[F/+] Fix Python fallback crashes and greatly improve shell completion#519Flaykky wants to merge 2 commits into
Flaykky wants to merge 2 commits into
Conversation
termenv.py:
- unix_detect_ansi_mode: `TERM`/`COLORTERM` can be absent from the
environment (CI, cron, some IDEs). `os.environ.get('TERM')` returns
None in that case, so subsequent `term.startswith(...)` and
`'256color' in term` calls raised AttributeError. Changed to
`os.environ.get('TERM') or ''` for both variables.
- unix_read_osc: same None dereference on line 99 (`term.startswith`).
Applied the same `or ''` guard.
- unix_read_osc: `code.lstrip(start)` was used to strip the OSC prefix,
but `str.lstrip` strips individual *characters*, not a prefix string,
so it could silently over-consume leading bytes of the actual payload.
Replaced with an explicit `code[len(start):]` slice after a
`startswith` check.
- windows_detect_ansi_mode: `map(int, platform.version().split('.'))`
crashes with ValueError/TypeError on non-standard version strings.
Wrapped in try/except with a safe fallback of `'rgb'`.
`int(os.environ.get('ANSICON_VER'))` raised when the var was unset or
non-numeric; guarded with `.isdigit()` before converting.
color_util.py:
- RGB.to_ansi: for `mode == 'ansi'` it forwarded to `to_ansi_16` which
is an unimplemented stub (`raise NotImplementedError`). For
`mode == 'default'` and any other unknown mode the function fell
through and returned None, causing TypeError when callers concatenated
the result into strings (e.g. in presets.py). Both cases now fall back
to `to_ansi_8bit`, which is a correct and safe 256-color degradation.
Return type annotation updated to `str`.
neofetch_util.py:
- ensure_git_bash: `git_path` returned by the `if_file(...)` chain can
be None when no Git Bash installation is found on Windows. The
subsequent `git_path.is_file()` then raised AttributeError instead of
printing the friendly error message. Fixed to `if not git_path or not
git_path.is_file():`.
- get_distro_ascii: `run_neofetch_cmd` can return None when neofetch is
missing or exits non-zero without raising. The following `.replace()`
call would then crash with AttributeError. Added an explicit None
guard that prints an error and exits cleanly.
- run(): the backend dispatcher had no else/fallback branch, so an
unknown or mistyped backend silently returned None and produced no
output. Added `raise ValueError(f"Unknown backend: {backend!r}")`.
types.py:
- BackendLiteral was `Literal["neofetch", "fastfetch"]`, missing
`"qwqfetch"` and `"fastfetch-old"` which are both accepted by the
argparse parser and the run() dispatcher. Stale types caused false
type-checker warnings for valid inputs. Updated to include all four
supported backends.
main.py:
- select_lightness: the prompt advertises `.45` and `0.45` as valid
decimal inputs, but the parser called `int(lightness)` first, which
raises ValueError for any non-integer float, landing in the error
path before the `float()` branch was ever reached. Reordered to:
handle `%` suffix first, then `float()` for everything else, dividing
by 100 only when the value exceeds 1.
- June/pride-month check: `os.isatty(sys.stdout.fileno())` raises
io.UnsupportedOperation when stdout is piped or replaced (e.g. during
testing or when output is redirected). Replaced with the pattern
already used in termenv.py: `hasattr(sys.stdout, 'isatty') and
sys.stdout.isatty()`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Previously the three dynamic completion functions in cli_options.rs
(complete_preset, complete_mode, complete_backend) only returned
candidates whose names *started with* the typed prefix, and returned
no hint text alongside each candidate.
This meant that, for example, typing `hyfetch -p trans<TAB>` found
`transgender` but typing `hyfetch -p gender<TAB>` found nothing, even
though `gender` uniquely identifies `transgender`. The interactive flag
picker (used during `hyfetch --config`) already performs substring
matching; the shell completion was inconsistent with it.
Changes:
- Extracted a shared `ranked_completions()` helper that drives all
three functions. It uses `str::contains` instead of `str::starts_with`
so any substring of a candidate name triggers a match.
- Results are sorted so that prefix matches appear before other
substring matches, then by position of the match within the name,
then alphabetically — identical ordering to the interactive picker's
`filter_flag_indices`.
- Each completion entry now carries a short human-readable description
("pride flag preset", "color mode", "fetch backend") surfaced by
shells that display hints next to candidates (e.g. zsh with
`complete_help`, fish).
- Added three unit tests under `#[cfg(feature = "autocomplete")]`:
complete_preset_substring – "gender" matches "transgender"
complete_preset_prefix_ranked_first – "trans" puts prefix match first
complete_preset_descriptions – every result has a non-empty hint
All five tests (including the pre-existing check_options and
models::test) pass with `cargo test -p hyfetch --features autocomplete`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hykilpikonna
reviewed
Jun 7, 2026
Comment on lines
-208
to
+210
| if mode == 'ansi': | ||
| return self.to_ansi_16(foreground) | ||
| # 'ansi' (16-color) is not yet implemented; fall back to 8bit which always works. | ||
| # 'default' and any unknown mode also fall through here as a safe degradation. | ||
| return self.to_ansi_8bit(foreground) |
Owner
There was a problem hiding this comment.
I think a better way would be to raise an explicit exception
If you write a typo in config, currently it will return None and somewhere will raise NPE to tell you that you've made a typo. After this PR, it'll just silently default to 256, and maybe take someone an hour to find the typo.
hykilpikonna
reviewed
Jun 7, 2026
Comment on lines
-246
to
+256
| .collect::<Vec<_>>() | ||
| .collect(); | ||
| // Prefix matches first (true sorts after false, so negate), then earliest position, then name. | ||
| matched.sort_by_key(|&(is_prefix, pos, name)| (!is_prefix, pos, name)); | ||
| matched | ||
| .into_iter() | ||
| .map(|(_, _, name)| (name.to_owned(), desc.clone())) | ||
| .collect() |
Owner
There was a problem hiding this comment.
I'm not that familar with completion, but, would this make the completion candidates sorted by name? I think they had an ordering before...
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.
Summary
Two groups of independent improvements:
Bug fixes — Python fallback (
hyfetch/)These affect the Python path (fallback on old Windows/macOS or when the
Rust binary is absent).
termenv.py—TERM/COLORTERMcan be unset in CI, cron, orsome IDEs;
os.environ.get('TERM')returnsNone, causingAttributeErroron every subsequentterm.startswith(...)/in termcall. Fixed with
or ''guards. Same fix applied inunix_read_osc.Also fixed
unix_read_osc's use ofstr.lstrip(start)(stripscharacters, not a prefix string) — replaced with
code[len(start):].Windows:
platform.version()parse now wrapped intry/except;ANSICON_VERguarded with.isdigit()beforeint().color_util.py—RGB.to_ansi('ansi')called the unimplementedto_ansi_16stub (raisesNotImplementedError);to_ansi('default')and unknown modes silently returned
None, causingTypeErrorincallers that concatenate the result. All non-rgb/8bit modes now fall
back to
to_ansi_8bitas a safe 256-color degradation.neofetch_util.py—ensure_git_bash:git_pathcan beNonewhen no Git Bash is found;
git_path.is_file()then raisesAttributeErrorinstead of showing the friendly error message. Fixedto
if not git_path or not git_path.is_file():.get_distro_ascii:run_neofetch_cmdcan returnNone— added a guard before.replace.run(): no fallback for unknown backends; addedraise ValueError(f"Unknown backend: {backend!r}").types.py—BackendLiteralwas missing"qwqfetch"and"fastfetch-old"(both accepted by argparse and the dispatcher).main.py—select_lightness:int('.45')raised before thefloat()branch, so decimal inputs like.45/0.45were rejecteddespite being documented as valid. Reordered parse:
%suffix first,then
float(), divide by 100 only when> 1. Pride-month check:replaced
os.isatty(sys.stdout.fileno())(raises on piped stdout)with
hasattr(sys.stdout, 'isatty') and sys.stdout.isatty().UX improvement - Rust shell completion (
crates/hyfetch/src/cli_options.rs)The interactive flag picker (
--config) already does substring +prefix-priority search, but the shell completion functions used only
starts_with, sohyfetch -p gender<TAB>never matchedtransgender.ranked_completions()used by all three completion fns.starts_withtocontains(substring).alphabetically — consistent with the interactive picker.
"color mode", "fetch backend") shown as a hint in zsh/fish.
cargo test -p hyfetch --features autocomplete.Example of completion system