Repository navigation
fix(powerline): keep Default Padding across the Powerline toggle, and report font install results correctly - #821
Open
eric-engberg wants to merge 8 commits into
Open
eric-engberg wants to merge 8 commits into
eric-engberg wants to merge 8 commits into
Conversation
…ack on Turning Powerline off and on again in Powerline Setup replaced a Custom theme with Nord Aurora. Nord Aurora's colors then drew over every widget's own colors, in the preview and in the status line, until the user went back to Themes and picked Custom again. Turning Powerline on gave the default theme to an unset theme and to 'custom' alike, treating both as the first time Powerline is enabled. An unset theme covers that case, since the settings default is no theme. 'custom' is only set when the user picks Custom, or copies a theme's colors onto the widgets with (c)ustomize, so it is now kept.
… off Turning Powerline on sets Default Padding to one space, so segment text doesn't touch the arrows. Turning it off only cleared the enabled flag, so the plain line kept that space for good: a line drawn as "Model | Ctx" came back as " Model | Ctx " after trying Powerline, and a padding the user had set, such as none or two spaces, was lost. That includes a fresh install, where Default Padding is unset. Turning Powerline on now remembers the padding it replaces in the Powerline settings, and turning it off puts that padding back and forgets it. When the padding was changed while Powerline was on, the change is the user's own and is kept. An unset padding is remembered as none, which renders the same. Settings saved before this change have nothing remembered, so turning Powerline off keeps the padding, as before.
On Windows, a Powerline font install that worked showed "Installed N Powerline fonts. Please restart your terminal..." in red, the error color. Powerline Setup picked green or red by looking for "success" in the message. The macOS and Linux message says "installed successfully", but the Windows one doesn't. The app passed on only the message and dropped the install's success flag, so the screen had nothing else to go on. The app now keeps the whole install result, and Powerline Setup colors the message by its success flag.
…r Git Installing fonts from Powerline Setup while offline, behind a proxy or behind a firewall said "Git is required to install Powerline fonts. Please install Git and try again." with git installed. The real error and the link to install the fonts by hand were hidden. The installer took any error whose text contained "git" to mean git is missing. execFileSync's error for a failed clone always starts with "Command failed: git clone ...", so every clone failure matched, whatever its cause. Only an ENOENT from starting git means git isn't installed, on Node and on Bun. That case keeps its message. Every other failure now shows the existing "Failed to install Powerline fonts: ... You can manually install fonts from: https://github.com/powerline/fonts" message with one line that says why, rather than the whole command line and its output. That is git's first "fatal:" or "error:" line, such as "fatal: unable to access 'https://github.com/powerline/fonts.git/': Could not resolve host: github.com", or else the first line of stderr that doesn't name the temp folder git clones into. That skips git's "Cloning into '<folder>'..." progress line in any language. git translates it and its "fatal:" prefix ("正克隆到 '<folder>'..." and "致命错误:" in Chinese, "Clonage dans '<folder>'..." and "fatal : " in French), but every translation quotes the folder. On a Mac without the Command Line Tools, where /usr/bin/git is a stub, the line shown is the stub's "xcode-select: note: No developer tools were found, requesting install." A command stopped by a signal says so, as in "git was stopped by SIGKILL". The existing cleanup test now fails the clone the way a missing git does.
# Conflicts: # src/utils/__tests__/powerline-settings.test.ts # src/utils/powerline-settings.ts
…ss on and off Turning Powerline on now leaves manual separators in the lines, and this branch keeps a chosen Custom theme and puts back the padding it replaced when Powerline goes off. Each was tested alone. A round trip through both helpers, and through (t) in Powerline Setup with the confirmation that manual separators bring up, checks all three come back together.
# Conflicts: # src/utils/__tests__/powerline-settings.test.ts # src/utils/powerline-settings.ts
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.
What
Three fixes in Powerline Setup. A fourth, keeping a Custom theme across the Powerline toggle, reached
mainas #793 and is gone from this diff.Model | Ctxafter trying Powerline, whatever the padding had been, including none on a fresh install.Why
Default Padding. Turning Powerline on sets
defaultPaddingto' ', so segment text doesn't touch the arrows. That part is deliberate, and an existing test asserts it. But the (t) branch that turns Powerline off only setenabled: false, and nothing recorded the earlier value. Onmain, enabling then disabling the way (t) does gave:" "" """" "" "" "" "" "The confirmation shown when Powerline is turned on mentions only separators, and with no manual separators there is no confirmation at all.
Font install message color. Powerline Setup picked the color with
fontInstallMessage.includes('success') ? 'green' : 'red'. The macOS and Linux message says "installed successfully". The Windows one,Installed ${installedCount} Powerline fonts. Please restart your terminal and select a Powerline font from your terminal settings., doesn't. The app passed on onlyresult.messageand droppedresult.success, so the screen could only guess from the wording. Not run on Windows: the code path shows it.Font install errors. The installer's catch block treated any error whose text contained
gitas git missing. execFileSync's error for a failed clone always starts withCommand failed: git clone ..., so every clone failure matched. With a stand-ingitfirst on PATH that fails a clone the way git does offline (exit 128,fatal: unable to access 'https://github.com/powerline/fonts.git/': Could not resolve host: github.com),installPowerlineFonts()onmainreturned{"success":false,"message":"Git is required to install Powerline fonts. Please install Git and try again."}under both Bun 1.4.2 and Node 26.10.0. The error it caught had nocode,status128, and git's output instderr. With git really missing, the error hascode: 'ENOENT'andpath: 'git'on both runtimes (Node:spawnSync git ENOENT; Bun:Executable not found in $PATH: "git"). Only that case means Git isn't installed. The manual install link was in the other message, so it was hidden too.How
PowerlineConfiggets an optionalpaddingToRestore.buildEnabledPowerlineSettingsstores the padding it replaces there, with an unset padding stored as''(they render the same). A newbuildDisabledPowerlineSettings, used by (t), turns Powerline off. It puts that padding back while Default Padding is still the' 'that turning on set, and always dropspaddingToRestore. A padding changed while Powerline was on is the user's own choice and stays. One line in docs/USAGE.md under Default Padding & Separators says so.FontInstallResult, now the return type ofinstallPowerlineFonts) instead of only its message. Powerline Setup'sfontInstallMessageprop takes that result, andgetFontInstallMessageColorpicks green or red fromsuccess.installPowerlineFonts: show "Git is required..." only for an ENOENT on startinggit. Every other failure gets the existing "Failed to install Powerline fonts: ... You can manually install fonts from: https://github.com/powerline/fonts" message. It now shows one line that says why, rather than the whole command line, temp folder path and output. That is the firstfatal:orerror:line git wrote to stderr, or else the first stderr line that doesn't name the temp folder git clones into. That skips git'sCloning into '<temp folder>'...progress line in any language. git translates that line and itsfatal:prefix (Chinese:正克隆到 '<temp folder>'...and致命错误:; French:Clonage dans '<temp folder>'...andfatal :), but every translation quotes the folder, so the check doesn't depend on git's wording. If a signal stopped the command, the message says so instead, as in "git was stopped by SIGKILL". An install.sh failure is shown the same way. No period is added after a line that already ends a sentence. The existing test "removes its folder when the clone fails" now fails the clone the way a missing git does, since a plain error no longer means git is missing.Left as is:
' ', also over a padding the user chose. Turning it off now puts that padding back.' ', as before."defaultPadding": ""instead of no key. Both render the same, and Global Overrides shows(none)for both./usr/bin/gitis a stub that fails instead of being missing, so that case doesn't get "Git is required...". It shows the stub's first line instead, "xcode-select: note: No developer tools were found, requesting install.", which gives the cause.LC_ALL=Cto make it English: the folder check finds git's reason in any language without it, git's reason in the user's own language says why just as well, andLC_ALLwould override every locale setting for git, not only its messages. In another language thefatal:/error:preference doesn't match, so the first line after the progress line is shown. That is git's error itself, or a line before it that says why, such as GitHub'sremote: Repository not found.Overlap with open upstream PRs: #718 also changes
src/utils/powerline.tsand conflicts with this one. #739 changes it too and merges cleanly with it.Demo
The Windows message color isn't recorded: it shows only on Windows, and the demo kit records on macOS and Linux. On those, the success message already says "successfully", so it looks the same before and after.
Default Padding. Plain: the plain line with no Default Padding, as on a fresh install. (t) turns Powerline on, (t) turns it off, then Global Overrides. Powerline: the same line after Powerline was turned on by this branch, so the settings remember no padding (
mainignores that key). (t) turns Powerline off, then Global Overrides. Before, the plain line comes back asModel: Claude | Ctx: 18.6k ...and Default Padding reads" ". After, it isModel: Claude | Ctx: 18.6k ...and Default Padding reads(none).Powerline
Before:
After:
Plain
Before:
After:
Font install with GitHub out of reach. Recorded with the stand-in
gitfirst on PATH, whose clone fails like git's does offline and which passes every other command to the real git, and withHTTPS_PROXYpointed at a closed port. Nothing is downloaded or installed. (i), then Yes. Before: "Git is required to install Powerline fonts. Please install Git and try again." After: "Failed to install Powerline fonts: fatal: unable to access 'https://github.com/powerline/fonts.git/': Could not resolve host: github.com. You can manually install fonts from: https://github.com/powerline/fonts".Powerline
Before:
After:
Plain
Before:
After:
Testing
mainafter fix(powerline): keep the Custom theme when Powerline is enabled again #793 made the Custom-theme fix there. This branch now keeps fix(powerline): keep the Custom theme when Powerline is enabled again #793's code and test for it; its own test that turns Powerline on, off and on again with a Custom theme still checks the theme along with the padding, and passes onmainfor the theme part.mainas of 2.2.34 (0c94df7): it merges cleanly.bun run lintis clean, andbun teston Linux CI gives 4424 pass, 0 fail (4403 onmain). The built CLI under Bun 1.4.4 and Node 26.13.0 agrees across runtimes and matchesmainin plain and Powerline mode, and the TUI opens and exits cleanly under both. It adds no JavaScript or Node API newer than Node 14, the build's target.powerline-settings.test.ts, 7 inPowerlineSetup.test.tsand 9 inpowerline-install.test.ts. All 20 fail onmain. Onmain,PowerlineSetup.test.tsdoesn't load, becausegetFontInstallMessageColordoesn't exist there. With that one test skipped, the other 6 fail one by one. "keeps a default padding changed since Powerline was turned on" fails onmainonly because the remembered padding isn't dropped, sincemainkeeps a changed padding too.bun run lintis clean.powerline-settings.test.ts(8) andPowerlineSetup.test.ts(15) pass. Every test inpowerline-install.test.tsfails onmain(2 of 2) and on this branch (11 of 11) with the same existing ESM spy error (vi.spyOn(os, 'homedir'): Cannot spy on export "homedir"). With that line removed locally, all 11 pass under Node.installPowerlineFonts()end to end under Bun 1.4.2 and Node 26.10.0, with stand-in gits first on PATH: the offline one gives the message above. One that prints the macOS stub's three lines (as they appear in this Mac's system libraries) and exits 1 gives "Failed to install Powerline fonts: xcode-select: note: No developer tools were found, requesting install. You can manually install fonts from: https://github.com/powerline/fonts". One that printsCloning into '<temp folder>'...and kills itself with SIGKILL gives "Failed to install Powerline fonts: git was stopped by SIGKILL. You can manually install fonts from: https://github.com/powerline/fonts". Two that print the offline failure as git does in Chinese (正克隆到 '<temp folder>'..., then致命错误:无法访问 'https://github.com/powerline/fonts.git/':Could not resolve host: github.com) and in French (Clonage dans '<temp folder>'..., thenfatal : impossible d'accéder à 'https://github.com/powerline/fonts.git/' : Could not resolve host: github.com) give "Failed to install Powerline fonts: 致命错误:无法访问 'https://github.com/powerline/fonts.git/':Could not resolve host: github.com. You can manually install fonts from: https://github.com/powerline/fonts" and the same with the French line. This Mac's git (Apple Git 2.50.1) is built without translations, so these are stand-ins, not a translated git. With git missing from PATH, both runtimes still return "Git is required...".mainin plain and Powerline mode under Bun 1.4.2 and Node 26.10.0, and both runtimes agree. The TUI opens and exits cleanly under both.