diff --git a/SPEC.md b/SPEC.md index c24db1b..3c2582f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -854,6 +854,176 @@ was the one property of a preset you could not set. boundary changes the stored order without changing what the menu shows. +### Phase 5a - Terminal parity ✅ done +Everything a mainstream terminal has that qtxterm did not. Grouped here +because the individual features are small; what was not small was discovering +that three of the shortcuts already documented had never worked. + +- [x] **Find in the scrollback** (`Ctrl+Shift+F`), on the vendored + `addon-search`. The bar lives *in the page*, not in a Qt widget above + the view: a Qt bar would take rows off the grid and reflow the shell's + output every time you searched. + - Two failures that unit tests passed straight through. xterm 5.5 gates + `registerMarker`/`registerDecoration` behind `allowProposedApi`, so the + addon threw *after* finding its matches, while selecting one - which + surfaced as "No results" on a query with three. And the two overview + ruler colours read as optional but are passed straight to + `registerDecoration`, where an undefined colour throws. + - The match highlight is a **translucent** tint, and that is not a + cosmetic choice. xterm draws search decorations *over* the glyphs, so an + opaque highlight erases the text it is pointing at. Reusing the theme's + `selectionBackground` looked safe - it is the one background a theme + guarantees its foreground on - and turned every match into a solid white + block on VS Code Dark High Contrast, whose selection colour is white. + Now yellow at 25%, the current match at 50% with a foreground-coloured + border. Caught by looking at a screenshot, not by a test. +- [x] **Clickable URLs**, on the vendored `addon-web-links`. Ctrl+click + (Cmd on macOS), following VS Code, Windows Terminal and iTerm2: a bare + click already places the cursor and starts a selection. + - Opens in the **system** browser, not a qtxterm browser tab, even though + the app has them. That is where the user's extensions, blocklists and + sessions are, and the URL is untrusted output. + - The scheme is checked again in Python. The addon's regex matches only + http/https, but it is not a security boundary - the text it ran against + came out of the terminal, which over SSH means it came from the remote + host - and `QDesktopServices.openUrl` will launch a registered handler + for any scheme. Tested against file, ms-msdt (Follina), javascript and + vbscript URLs. +- [x] **Copy/paste, font zoom and tab-by-number** shortcuts, plus + **close-on-exit** and a **background image**. + +#### Focus after a split was landing on the tab bar - fixed +Reported as "pane navigation moves between tabs". It was not the navigation: +after `split_active()` (and after `addTab`) Qt left keyboard focus on the +**QTabBar**, which handles Left/Right by switching tabs. So a freshly split +pane could not be typed into until it was clicked, and arrows moved tabs. + +- [x] `PaneWidget.focus_pane()`, called after a split, a new tab, and a pane + close. For a terminal it takes two steps: `setFocus()` on the view + moves Qt's focus off the tab bar, and `term.focus()` moves it again + *inside* the page to the hidden textarea xterm reads from. Without the + second the pane looks focused and types nowhere. A pane split a moment + ago has no page yet, so the request is remembered and replayed from + `_on_script_loaded`. +- [x] Alt+Arrow pane navigation ranks candidates by how much their edges + **overlap**, not by distance between centres. Measured: with a tall + pane left of two stacked right, the two candidates' centres sat 138 and + 139 pixels off axis - a one-pixel coin flip that would flip again if + the splitter moved. + +#### Punctuation shortcuts fail in two opposite ways - fixed +The `Alt+Shift` splits were documented from Phase 4i and could never have +fired from a keyboard. Both classes are now covered by registering every +spelling. + +| Chord | Qt matches | Keyboard sends | +|---|---|---| +| Alt+Shift+equals | `Key_Equal` | `Key_Plus`, because Shift+equals is plus | +| Alt+Shift+minus | `Key_Minus` | `Key_Underscore` | +| Ctrl+Shift+underscore | `Key_Underscore` | `Key_Minus` - with Ctrl held the character is a control code, so Qt cannot derive the shifted character from the layout and reports the base key | + +Worth knowing for the next such bug: **`QTest.keyClick` cannot catch this.** +It hands Qt an event you constructed, so it only ever confirms that a binding +matches the event you invented. An earlier "all six chords work" measurement +was made that way and was worthless. `SendInput` is the honest test, and it +is unavailable in some environments - it returned 0 here even with the window +confirmed foreground. + +#### Shortcuts are per platform, not translated - `shortcuts.py` +Qt maps `Ctrl` to Command on macOS, `Meta` to Control and `Alt` to Option. +That does half the job and ruins the other half, so the table spells out both +sides: + +- On Windows and Linux the shell owns Ctrl+letter, so the app takes + `Ctrl+Shift`. On macOS the shell uses Control and Command is free, so the + binding is a plain Cmd+letter - translating `Ctrl+Shift+T` would give + Cmd+Shift+T, a different gesture entirely. +- `Ctrl+Tab` on macOS becomes Cmd+Tab, the OS application switcher, so + next-tab is spelled `Meta+Tab` there. +- `Ctrl+C` on macOS is Cmd+C and is genuinely copy; the interrupt is + `Meta+C` and is deliberately left unbound. A test asserts it stays that + way. +- `shortcuts.conflicts()` exists because a collision is invisible: two + QShortcuts sharing a sequence makes Qt fire **neither**. A test calls it on + both platforms rather than trusting the table to have been read carefully. + It caught the split-down chord being a tempting second spelling for + zoom-out. + +#### Close-on-exit is three settings, not a checkbox +A shell you exited on purpose should take its pane with it; a shell that +*died* has usually printed why, and closing the pane throws that away exactly +when you needed to read it. Default is the middle option, matching Windows +Terminal's closeOnExit. Closing is deferred a turn of the event loop - +deleting the widget that owns the object currently emitting is how you get a +crash rather than a closed pane - and the pane is re-checked at that point, +since the tab can be gone by then. + +#### Background image +The theme colour becomes a **veil** over the image rather than the ground: +the image is the bottom layer, with a flat wash of the theme background over +it at (100 minus strength). Default strength 30, because a photograph at full +strength behind text is unreadable and the first thing someone tries should +still work as a terminal. `allowTransparency` is set at construction, and +xterm's own background goes transparent only when an image is set, so the +no-image case renders exactly as before. + +#### Shortcuts are rebindable, and only the differences are stored +Added because no default table can be right everywhere, and the reason is not +taste. A tiling window manager that owns Alt+Arrow, a desktop that has claimed +a chord, a shell binding somebody depends on - none of these are visible from +inside qtxterm, and all of them take the key before the app sees it. Rebinding +makes the defaults a starting point rather than a verdict. + +The alternative considered first was adding a second default chord per action +as a fallback, which is what prompted the question this settles: **is binding +several chords to one action good practice?** The distinction that matters: + +- A **compatibility hedge** is two spellings of one physical gesture where + only one can ever fire - Alt+Shift+= and Alt+Shift++ are the same keypress. + Free: no keyspace consumed, nothing shadowed, nothing extra to learn. +- A **true alias** is two different gestures for one action - Ctrl+= and + Ctrl++, or Ctrl+Shift+C and Ctrl+Insert. Each costs a chord permanently, and + in a terminal the keyspace is scarce because the shell owns most of it. + +Hedge freely; alias only against evidence. Measured at the time: 26 actions +resolved to 44 sequences on Windows/Linux against 29 on macOS, and the +tab-number slots alone (Alt+N *and* Ctrl+Alt+N) accounted for roughly half the +aliasing. Adding Ctrl+Shift+Arrow as a speculative Linux fallback was rejected +on those grounds - it would have been a "just in case" alias, and rebinding is +the honest answer to environmental capture. + +Three decisions in the implementation: + +- **Only overrides are written.** Saving the resolved table would freeze + today's defaults into every config file, so a later version that improved a + binding or added an action would never reach anyone who had opened the + editor once. An untouched action follows the defaults forever. +- **A conflict is refused, not accepted.** Two QShortcuts sharing a sequence + makes Qt fire *neither* - it reports the ambiguity and gives up - so a + last-one-wins policy would silently disable both actions. The store rejects + the save and names the action already holding the chord. +- **Shortcuts are rebuilt, not patched.** Working out which QShortcuts a + changed table implies is more code than making them all again, and the set + is small. The previous set is disposed first: a left-behind QShortcut keeps + firing, and once its replacement exists the two are ambiguous - which is the + failure above, self-inflicted. + +`shortcuts.py` stays a pure table with no Qt storage in it; `keybindings.py` +layers overrides on top, and `terminal_tabs` resolves through the store when +it has one and straight from the table when it does not, which is what tests +and embedders get. + +An empty binding list is a real setting, distinct from resetting: an action +nobody wants a key for is a legitimate thing to ask for, and the difference is +"no shortcut" versus "the default shortcut". + +#### Documentation is tested, not proofread +`tests/test_usage_docs.py` asserts that every shortcut the app binds appears +in USAGE.md, and that the guide survives `QTextBrowser.setMarkdown` - whose +escaping rules are not GitHub's, so the file being correct is no evidence the +dialog is. It was written after the guide was found advertising chords that +could not fire, and after a pipe character silently ate a table cell. + ## Open Questions / Deferred ### Start-up latency - measured, not yet decided @@ -953,36 +1123,64 @@ with no stop is exactly today's behaviour. No visible "advanced" tier - one job type with optional fields, because two tiers is two things to learn and a migration when a simple job later needs a window. -### Ctrl-C does not reach child processes on Windows - bug, not yet fixed -Found while designing the above, and **independent of cron**: pressing Ctrl-C -in any qtxterm terminal does not stop a running command on Windows. - -`pywinpty`'s `sendintr()` is literally `write("")`, which is what typing -Ctrl-C already does. Measured, spawning `ping -t` and counting replies: +### Ctrl-C on Windows - the reported bug was a measurement artifact +Re-measured during Phase 5a, and the conclusion is the opposite of what this +document said before: **Ctrl-C works.** Pressing it in a cmd or PowerShell tab +stops a running child, in a normally launched qtxterm. -| Backend | child started | after `` | after closing | -|---|---|---|---| -| **ConPTY** (what we use) | running | **still running** | killed | -| WinPTY (legacy) | running | killed | killed | +What made it look broken is worth recording, because it is an easy trap and it +wasted a long investigation. -At an idle prompt the shell echoes `^C`, which makes it look like it worked; -a running child never sees it. Closing the terminal kills the process under -both backends, so that is the only stop that currently works. +**Windows makes the "ignore Ctrl+C" state inheritable.** A process whose +parent called `SetConsoleCtrlHandler(NULL, TRUE)` inherits the ignore, and so +does everything *it* spawns. Automation runners, CI agents and some launchers +set it as a matter of course. When qtxterm is started from such a process the +flag travels down the whole chain - qtxterm, its ConPTY, the shell in the tab, +and the shell's children - so nothing in any terminal responds to Ctrl+C. The +same build launched normally is fine. -Three ways out: +Measured, one script, two trials, the only difference being the flag: -1. `GenerateConsoleCtrlEvent` - attach to the child console, raise - CTRL_C_EVENT, detach. The correct mechanism, and it fixes interactive - Ctrl-C too. Our own process must ignore the event first or it takes the - app down with it. -2. Switch to the WinPTY backend - works immediately, but it is the - deprecated emulation layer with an extra agent process and worse - fidelity. Bad trade for the terminal's quality. -3. Leave it, and stop things by closing the tab. - -Preferred: 1. Linux is unaffected - a real SIGINT to the foreground process -group is straightforward there, and untested only because the bug is -Windows-specific. +| Condition | Child after Ctrl-C | +|---|---| +| as launched by an automation shell | still running | +| after `SetConsoleCtrlHandler(NULL, FALSE)` | **stopped** | + +Two earlier findings were real observations with the wrong explanation +attached, and both dissolve once the flag is understood: + +- **Git Bash appeared immune.** It is: MSYS2 implements POSIX signals in its + own runtime and raises SIGINT for the foreground process group without ever + consulting the Windows console control path, so an inherited console flag + cannot affect it. That is why it kept working while cmd and PowerShell did + not - not evidence of a ConPTY defect. +- **`AttachConsole` + `GenerateConsoleCtrlEvent` reported success and killed + nothing.** Expected, once the target is ignoring the event. + +The ConPTY-versus-WinPTY table previously recorded here should be treated as +unverified: it may well have been taken under the same inherited flag, and it +attributes to ConPTY a behaviour that reproduces just as readily without it. + +Genuinely useful things learned along the way, kept because they are true +regardless: + +- `AttachConsole` **resets** the Ctrl+C ignore flag, so + `SetConsoleCtrlHandler(NULL, TRUE)` must be called *after* attaching, not + before. Getting this backwards takes the calling process down with the + event. Doing console surgery in a throwaway helper process avoids the + question entirely. +- The standard handles are stale after `AttachConsole`; `CONIN$` has to be + reopened with `CreateFile` before `GetConsoleMode` will work. +- The pseudoconsole's input mode already has `ENABLE_PROCESSED_INPUT` + (measured 0x01f7), so the "processed input is off" theory is dead in any + case. + +**Testing lesson, which is the durable part.** Every measurement above was +taken from a harness that silently changed the thing being measured. A test +environment that disables Ctrl+C cannot be used to test Ctrl+C, and nothing in +the output said so - the app simply looked broken. Where a behaviour depends +on process or console state inherited from the launcher, the only trustworthy +check is the application started the way a user starts it. ### Stable `Preset.id` - proposed, low priority, not implemented Give every preset an `id: str` (uuid4 hex), assigned in `__post_init__` when diff --git a/scripts/smoke_test.py b/scripts/smoke_test.py index ddca2aa..54a934b 100644 --- a/scripts/smoke_test.py +++ b/scripts/smoke_test.py @@ -30,6 +30,8 @@ "assets/xterm/xterm.js", "assets/xterm/xterm.css", "assets/xterm/addon-fit.js", + "assets/xterm/addon-search.js", + "assets/xterm/addon-web-links.js", "assets/USAGE.md", "assets/logo.ico", ] diff --git a/src/qtxterm/appearance.py b/src/qtxterm/appearance.py index c3085f1..4d11026 100644 --- a/src/qtxterm/appearance.py +++ b/src/qtxterm/appearance.py @@ -10,9 +10,16 @@ _FONT_FAMILY_KEY = "appearance/fontFamily" _FONT_SIZE_KEY = "appearance/fontSize" _SCROLLBACK_KEY = "appearance/scrollback" +_BACKGROUND_IMAGE_KEY = "appearance/backgroundImage" +_BACKGROUND_OPACITY_KEY = "appearance/backgroundOpacity" DEFAULT_FONT_FAMILY = "Consolas" DEFAULT_FONT_SIZE = 14 +# Shared by the Preferences spin box and the zoom shortcuts, so the two +# cannot drift apart - zooming past a size the dialog refuses to show would +# leave a preference the user could see but not edit back. +MIN_FONT_SIZE = 6 +MAX_FONT_SIZE = 72 # xterm.js's own default, kept as ours so the setting starts where the # terminal already was. Every line held is memory that a terminal left open @@ -24,6 +31,15 @@ MIN_SCROLLBACK = 0 MAX_SCROLLBACK = 100_000 +# How strongly a background image shows through, as a percentage. 0 hides it +# entirely (the theme's own background, i.e. no image at all) and 100 shows it +# untouched. The default is deliberately well below 100: a photograph at full +# strength behind text is unreadable, and someone trying the feature for the +# first time should see something that still works as a terminal. +MIN_BACKGROUND_OPACITY = 0 +MAX_BACKGROUND_OPACITY = 100 +DEFAULT_BACKGROUND_OPACITY = 30 + @dataclasses.dataclass class Appearance: @@ -31,6 +47,11 @@ class Appearance: font_family: str = DEFAULT_FONT_FAMILY font_size: int = DEFAULT_FONT_SIZE scrollback: int = DEFAULT_SCROLLBACK + # Absolute path to an image, or "" for none. Stored as the path the + # user picked rather than a copy, so replacing the file on disk + # changes the background without touching the setting. + background_image: str = "" + background_opacity: int = DEFAULT_BACKGROUND_OPACITY @property def theme(self) -> Theme: @@ -57,11 +78,21 @@ def _load(self) -> Appearance: theme_name = default_theme_name() font_family = self._settings.value(_FONT_FAMILY_KEY, DEFAULT_FONT_FAMILY) font_size = int(self._settings.value(_FONT_SIZE_KEY, DEFAULT_FONT_SIZE)) + font_size = max(MIN_FONT_SIZE, min(font_size, MAX_FONT_SIZE)) scrollback = int(self._settings.value(_SCROLLBACK_KEY, DEFAULT_SCROLLBACK)) + background_image = self._settings.value(_BACKGROUND_IMAGE_KEY, "") or "" + background_opacity = int( + self._settings.value(_BACKGROUND_OPACITY_KEY, DEFAULT_BACKGROUND_OPACITY) + ) return Appearance( theme_name=theme_name, font_family=font_family, font_size=font_size, + background_image=background_image, + background_opacity=max( + MIN_BACKGROUND_OPACITY, + min(background_opacity, MAX_BACKGROUND_OPACITY), + ), # Clamped on the way in: a hand-edited ini shouldn't be able to # ask xterm.js for a negative buffer. scrollback=max(MIN_SCROLLBACK, min(scrollback, MAX_SCROLLBACK)), @@ -73,4 +104,6 @@ def save(self, appearance: Appearance) -> None: self._settings.setValue(_FONT_FAMILY_KEY, appearance.font_family) self._settings.setValue(_FONT_SIZE_KEY, appearance.font_size) self._settings.setValue(_SCROLLBACK_KEY, appearance.scrollback) + self._settings.setValue(_BACKGROUND_IMAGE_KEY, appearance.background_image) + self._settings.setValue(_BACKGROUND_OPACITY_KEY, appearance.background_opacity) self.changed.emit() diff --git a/src/qtxterm/assets/USAGE.md b/src/qtxterm/assets/USAGE.md index d0c860c..20d0952 100644 --- a/src/qtxterm/assets/USAGE.md +++ b/src/qtxterm/assets/USAGE.md @@ -4,6 +4,10 @@ A tabbed terminal with one-click command buttons and reusable command presets. ## Terminals and tabs +Shortcuts below are the Windows and Linux ones. macOS uses Command in +place of Ctrl and drops the Shift - `Cmd+T`, `Cmd+W`, `Cmd+F`. The full +side-by-side list is under [Keyboard shortcuts](#keyboard-shortcuts). + | Action | How | |---|---| | New tab (default shell) | `Ctrl+Shift+T`, or the `+` button at the right of the tab bar | @@ -11,9 +15,14 @@ A tabbed terminal with one-click command buttons and reusable command presets. | New browser tab | **File → New Browser** | | Close tab | `Ctrl+Shift+W`, or the `x` on the tab | | Next / previous tab | `Ctrl+Tab` / `Ctrl+Shift+Tab` | +| Go to tab 1-8, or the last | `Alt+1` ... `Alt+9`, or `Ctrl+Alt+1` ... | | Rename a tab | Double-click the tab | -| Split the pane | `Alt+Shift+=` (right) / `Alt+Shift+-` (down), or right-click → **Pane → Split** | +| Find in the scrollback | `Ctrl+Shift+F` | +| Copy / paste | `Ctrl+Shift+C` / `Ctrl+Shift+V` | +| Bigger / smaller / default text | `Ctrl+=` / `Ctrl+-` / `Ctrl+0` | +| Split the pane | Right-click → **Pane → Split**, or see [Split panes](#split-panes) | | Close a pane | `Alt+Shift+W`, or right-click → **Pane → Close** | +| Move the keyboard between panes | `Alt+←` `Alt+→` `Alt+↑` `Alt+↓` | | Move a pane | Right-click → **Pane → Move Left/Right** (or Up/Down) | | Pull a pane into its own tab | Right-click → **Pane → Move to New Tab** | @@ -46,9 +55,15 @@ button isn't available there - Qt only draws it alongside existing tabs. ## Split panes A tab can hold several panes side by side. Right-click a terminal and pick -**Pane → Split Right** or **Split Down**, or use `Alt+Shift+=` / -`Alt+Shift+-`. Everything that rearranges panes lives under that one **Pane** -group. +**Pane → Split Right** or **Split Down**, or use a keyboard chord - +there are two pairs and either works: + +- `Ctrl+Shift+|` splits **right** - a vertical bar for a vertical divider +- `Ctrl+Shift+_` splits **down** - an underscore for a horizontal one +- `Alt+Shift++` splits right, `Alt+Shift+-` splits down, matching Windows + Terminal + +Everything that rearranges panes lives under that one **Pane** group. Splits nest, so you can build columns of rows. Drag the divider to resize. Browser panes split too, and a split gives you **another pane of the same @@ -56,6 +71,15 @@ kind** - splitting a browser gives a browser, splitting a terminal gives a terminal. In a browser pane the shortcuts are the only route: right-clicking a web page shows Chromium's own menu, which you want for links and images. +`Alt+←` `Alt+→` `Alt+↑` `Alt+↓` move the keyboard between panes. They go by +where panes actually sit on screen, not by the order they were created, so +`Alt+→` lands on the pane genuinely to the right even in a nested split. There +is no wraparound - from the rightmost pane, `Alt+→` stays put. + +A new pane takes the keyboard as soon as it opens, whether it came from a +split or a new tab, so you can type into it straight away without clicking +first. + The pane you last clicked or typed in is the **active** one, outlined in the highlight colour whenever a tab has more than one. That outline matters: sidebar buttons, the Command menu and Selection Actions all go to the active @@ -73,8 +97,91 @@ commands cover the cases that actually come up. **Pane → Close** (`Alt+Shift+W`) closes just that terminal; closing the last pane closes the tab. `Ctrl+Shift+W` still closes the whole tab, panes and -all. `Alt+Shift` chords are used rather than `Ctrl+Shift` because shells and -full-screen apps rarely bind them. +all. `Alt+Shift+W` rather than a `Ctrl` chord, because closing a pane +sits next to the `Alt+Shift` splits above it. + +## Keyboard shortcuts + +Every shortcut, on each platform. The two columns differ more than a +find-and-replace would suggest, and the reason is worth a sentence: + +- **On Windows and Linux the shell owns `Ctrl`+letter.** `Ctrl+C` interrupts, + `Ctrl+W` deletes a word, `Ctrl+F` moves forward a character. So qtxterm's + own actions take `Ctrl+Shift`, exactly as Windows Terminal, GNOME Terminal + and VS Code's terminal do. +- **On macOS the opposite holds.** The shell uses Control and Command is + free, so the binding is plain `Cmd`+letter, like every other Mac app. In + particular `Cmd+C` is copy while Control+C still interrupts, because they + are different keys. + +| Action | Windows / Linux | macOS | +|---|---|---| +| New tab | `Ctrl+Shift+T` | `Cmd+T` | +| Close tab | `Ctrl+Shift+W` | `Cmd+W` | +| Next tab | `Ctrl+Tab` | `Ctrl+Tab` or `Cmd+Shift+]` | +| Previous tab | `Ctrl+Shift+Tab` | `Ctrl+Shift+Tab` or `Cmd+Shift+[` | +| Go to tab 1-8, or the last with 9 | `Alt+1`..`Alt+9` or `Ctrl+Alt+1`..`9` | `Cmd+1`..`Cmd+9` | + +Where two chords are listed they both work on purpose. `Alt+1` is GNOME +Terminal's and `Ctrl+Alt+1` is Windows Terminal's, and people arrive with +one or the other already in their fingers. +| Find | `Ctrl+Shift+F` | `Cmd+F` | +| Copy | `Ctrl+Shift+C` or `Ctrl+Insert` | `Cmd+C` | +| Paste | `Ctrl+Shift+V` or `Shift+Insert` | `Cmd+V` | +| Bigger text | `Ctrl+=` or `Ctrl+Shift+=` | `Cmd+=` or `Cmd+Shift+=` | +| Smaller text | `Ctrl+-` | `Cmd+-` | +| Default text size | `Ctrl+0` | `Cmd+0` | +| Split right | `Alt+Shift++` | `Cmd+D` | +| Split down | `Alt+Shift+-` | `Cmd+Shift+D` | +| Close pane | `Alt+Shift+W` | `Cmd+Shift+W` | +| Move between panes | `Alt+←` `Alt+→` `Alt+↑` `Alt+↓` | `Cmd+Opt+←` and friends | +| Follow a link | `Ctrl+click` | `Cmd+click` | + +On Windows and Linux the splits have a second pair, `Ctrl+Shift+|` for right +and `Ctrl+Shift+_` for down, which read as what they do - a vertical bar for +a vertical divider, an underscore for a horizontal one. + +Two macOS choices are worth calling out. Next tab is a physical `Ctrl+Tab` +there, not `Cmd+Tab`, which belongs to the OS application switcher. And +`Cmd+D` / `Cmd+Shift+D` for splitting come from iTerm2 rather than from the +Windows chords, since that is what Mac terminal users already have in their +fingers. + +### Changing a shortcut + +**File -> Keyboard Shortcuts...** rebinds any of them. Pick an action, press +the keys you want, and press **Add**. **Remove** drops a chord, **Reset** puts +one action back to its default, and **Reset All** puts back the lot. Actions +you have changed are shown in bold. + +An action can hold more than one chord - that is why several of them ship with +two - and it can hold none at all, if you would rather have the key back for +the shell. + +Two things the editor will not let you do, both for the same reason. A chord +already used by another action is refused, naming the action holding it; and a +chord you type replaces nothing silently. Qt fires **neither** of two shortcuts +that share a chord, so quietly accepting a duplicate would break both actions +with nothing to show for it. + +Changes apply immediately - there is no restart, and open tabs pick the new +chord up at once. + +Only the shortcuts you actually change are saved, so anything you leave alone +keeps following the defaults, including in later versions that improve them. +The file is JSON and hand-editable, beside your presets: + +- Windows - `%LOCALAPPDATA%\qtxterm\keybindings.json` +- macOS - `~/Library/Application Support/qtxterm/keybindings.json` +- Linux - `~/.config/qtxterm/keybindings.json` + +If a chord in that file cannot be read it is ignored rather than stopping the +app, and the action falls back to its default. + +This is also the answer to a shortcut that never arrives. A tiling window +manager that owns `Alt+Arrow`, or a desktop that has claimed a chord for +itself, takes the key before qtxterm ever sees it - no default table can +predict that, so rebind it to something free. ## Browser tabs @@ -122,7 +229,17 @@ item flips back to unchecked so you can bring it back. ## Copy and paste -Right-click in a terminal for **Copy** and **Paste**. +`Ctrl+Shift+C` and `Ctrl+Shift+V`, or right-click for **Copy** and **Paste**. +`Ctrl+Insert` and `Shift+Insert` work too. On macOS it is plain `Cmd+C` and +`Cmd+V`. + +`Ctrl+C` is deliberately left alone on Windows and Linux: it is the +interrupt, and a terminal that stole it to mean copy would be unable to stop +a running command. macOS has no such clash, because copy is `Cmd+C` there +while the interrupt is Control+C - two different keys. + +Copying with nothing selected does nothing at all rather than emptying the +clipboard, which matters most on macOS where the binding is a bare `Cmd+C`. - **Copy** takes the text you've selected with the mouse. It's greyed out when nothing is selected. @@ -131,6 +248,62 @@ Right-click in a terminal for **Copy** and **Paste**. clipboard text is handed to the terminal as a paste, not as typing, so shells and editors that use bracketed paste treat it correctly. +## Find in the scrollback + +`Ctrl+Shift+F` opens a find bar in the top-right corner of the active +terminal. It searches the whole scrollback, not just the lines on screen, so +it will find something that scrolled past a thousand lines ago - as far back +as your scrollback setting keeps. + +| Key | What | +|---|---| +| `Ctrl+Shift+F` | Open the find bar (again to re-focus it) | +| `Enter` | Next match | +| `Shift+Enter` | Previous match | +| `Esc` | Close, clear the highlights, and put the cursor back in the terminal | + +`Aa` makes the search case-sensitive, `.*` treats what you typed as a regular +expression. The counter reads `3 of 17`, or `No results` with the box outlined +in red. + +Every match is tinted, and the one you are on is brighter with an outline +around it. Both are drawn in the theme's own colors, so the bar and the +highlights follow whatever theme you have set - including a theme change while +the bar is open. + +Typing extends the current match rather than jumping ahead on every keystroke, +so searching for `error` doesn't walk you through three matches on the way to +finishing the word. + +`Ctrl+Shift+F` rather than `Ctrl+F` because `Ctrl+F` is forward-char in +bash and readline, and is bound to something in most full-screen apps. Find +does nothing in a browser pane - there is no scrollback to search, and it +won't quietly search a terminal you aren't looking at. + +## Clickable links + +A URL in the output is a link. Hover it and it underlines, with a tip showing +where it goes; **Ctrl+click** (Cmd+click on macOS) opens it in your normal +browser. + +Ctrl rather than a plain click, matching VS Code's terminal, Windows Terminal +and iTerm2: an ordinary click already places the cursor and starts a +selection, and terminal output is full of URLs you did not mean to visit. + +Only `http://` and `https://` are linkified, and only those are ever opened. +A bare `example.com` stays plain text. That is deliberate rather than +fussiness - what the terminal prints is not necessarily yours: over SSH it is +whatever the remote host chose to print. Restricting the schemes keeps a +printed line from launching a local handler. + +The tip shows the whole target, which is worth reading before you click: a +link can wrap across two rows or run off the edge of the terminal, so the +text under your cursor is not always the whole URL. + +Links open in your system browser rather than a qtxterm browser tab. That is +where your extensions, blocklists and logged-in sessions already live, and +it is the safer home for a URL that arrived as untrusted output. + ## Selection Actions Select text in a terminal, right-click, and pick **Selection** to run @@ -230,13 +403,34 @@ Changes save immediately, and the sidebar and both menus refresh straight away. Presets are stored as JSON, so you can hand-edit or back them up: - Windows - `%LOCALAPPDATA%\qtxterm\presets.json` -- Linux - `~/.config/qtxterm/presets.json` - macOS - `~/Library/Application Support/qtxterm/presets.json` +- Linux - `~/.config/qtxterm/presets.json` + +## Background image + +**File -> Preferences... -> Background image** puts a picture behind the +terminal. **Image strength** controls how much of it shows through. + +The theme colour is laid over the image as a veil rather than replaced by it, +so lowering the strength dims the picture toward your theme's normal +background. The default is 30%, because a photograph at full strength behind +text is unreadable - start there and raise it until it stops being +comfortable. + +**The image spans the tab, not each pane.** Split a tab three ways and you +get one continuous picture with the dividers cutting across it, rather than +the same image repeated in every pane. Each tab shows the whole image again. + +The path is stored, not a copy of the file, so replacing the image on disk +changes the background without touching the setting. Point it at a file that +no longer exists and you simply get a normal terminal rather than a broken +one. **Clear** removes it. ## Preferences -**File → Preferences...** sets the default shell, color theme, font, and -font size. +**File → Preferences...** sets the default shell, what happens when a shell +exits, the color theme, font, font size, scrollback, and the order of the +right-click menu. ### Default shell @@ -250,6 +444,25 @@ shell you pick there regardless of this setting. If the chosen shell later disappears - a WSL distro you removed - new tabs quietly fall back to the system default rather than failing to open. +### When a shell exits + +What happens to a pane once its shell finishes: + +| Setting | What it does | +|---|---| +| Close it, unless the shell failed | The default. A shell you exited on purpose takes its pane with it; one that died leaves the pane open | +| Always close it | Even when the shell failed | +| Leave it open | What qtxterm did before this setting existed | + +The default is the middle ground on purpose. Exiting a shell yourself means +you are finished with that pane, so keeping it costs a second keystroke. But +a shell that *died* has usually printed why, and closing its pane throws that +away exactly when you wanted to read it. + +Only the pane closes, not the tab around it - unless it was the last pane, in +which case the tab goes too. The window still outlives its terminals either +way. + ### Appearance | Theme | Look | @@ -266,6 +479,14 @@ leaves the native look alone. Changes apply immediately to every open tab, and are remembered for next time. +`Ctrl+=` and `Ctrl+-` resize the text without opening this dialog, and +`Ctrl+0` puts it back to the default. `Ctrl+Shift+=` zooms in too - it is +the same key with Shift held, which is how most people press "plus", and +every browser accepts both. There is only one stored size, so +zooming *is* editing the preference - which is why `Ctrl+0` returns to the +default rather than to whatever the dialog last held, since otherwise it +would have nothing to mean. + ## What else is remembered The window's size and position, and whether the Commands sidebar is showing, @@ -273,8 +494,8 @@ are restored the next time you open qtxterm - alongside the appearance settings above. They live next to your presets: - Windows - `%LOCALAPPDATA%\qtxterm\window_state.ini` -- Linux - `~/.config/qtxterm/window_state.ini` - macOS - `~/Library/Application Support/qtxterm/window_state.ini` +- Linux - `~/.config/qtxterm/window_state.ini` ## How multiline presets run diff --git a/src/qtxterm/assets/terminal.html b/src/qtxterm/assets/terminal.html index 8348505..1c936aa 100644 --- a/src/qtxterm/assets/terminal.html +++ b/src/qtxterm/assets/terminal.html @@ -18,13 +18,119 @@ resolve CSS custom properties inside ::-webkit-scrollbar-* pseudo elements, so a var() rule silently falls back and never follows the theme. */ + + /* Find bar. Lives in the page rather than in a Qt widget above the view so + that showing it doesn't resize the terminal - a Qt bar would take rows + off the grid, reflowing the shell's output every time you searched. + Overlaid, so it costs the terminal nothing. + + Colors come from the theme as custom properties set by terminal.js + (applyFindTheme). Only the four roots are injected; every shade below is + mixed off them, so a new theme needs no new variables. */ + #find-bar[hidden] { display: none; } + #find-bar { + position: fixed; + top: 0; + /* Clear of the scrollbar (8px) so the bar never sits on top of it. */ + right: 14px; + z-index: 10; + display: flex; + align-items: center; + gap: 4px; + padding: 5px 6px; + /* Lifted off the terminal ground, or the bar reads as output. */ + background: color-mix(in srgb, var(--find-fg) 10%, var(--find-bg)); + border: 1px solid color-mix(in srgb, var(--find-fg) 32%, var(--find-bg)); + border-top: none; + border-radius: 0 0 5px 5px; + box-shadow: 0 2px 10px rgba(0, 0, 0, 0.35); + /* The UI font, not the terminal's: this is chrome, not output. */ + font: 12px/1.4 system-ui, "Segoe UI", sans-serif; + color: var(--find-fg); + } + #find-input { + width: 180px; + padding: 3px 6px; + font: inherit; + color: var(--find-fg); + background: var(--find-bg); + border: 1px solid color-mix(in srgb, var(--find-fg) 32%, var(--find-bg)); + border-radius: 3px; + outline: none; + } + #find-input:focus { border-color: var(--find-accent); } + #find-bar.no-results #find-input { border-color: var(--find-error); } + #find-count { + min-width: 66px; + text-align: right; + opacity: 0.75; + /* So the width doesn't twitch as the index counts up. */ + font-variant-numeric: tabular-nums; + } + #find-bar button { + min-width: 22px; + height: 22px; + padding: 0 4px; + font: inherit; + line-height: 1; + color: var(--find-fg); + background: transparent; + border: 1px solid transparent; + border-radius: 3px; + cursor: pointer; + } + #find-bar button:hover { + background: color-mix(in srgb, var(--find-fg) 18%, transparent); + } + #find-bar button.active { + background: color-mix(in srgb, var(--find-accent) 30%, transparent); + border-color: var(--find-accent); + } + + /* Hovering a URL shows what it points at and how to follow it. + Both halves matter: a link can be wrapped across two rows or run off the + edge of the terminal, so the text under the cursor is not necessarily + the whole target - and the modifier is not guessable. Positioned from + JS, since it follows the pointer. */ + #link-tip[hidden] { display: none; } + #link-tip { + position: fixed; + z-index: 11; + max-width: 60ch; + padding: 4px 7px; + /* Breaks anywhere, because a URL has no spaces to wrap at and would + otherwise push the tip wider than the window. */ + overflow-wrap: anywhere; + font: 12px/1.4 system-ui, "Segoe UI", sans-serif; + color: var(--find-fg); + background: color-mix(in srgb, var(--find-fg) 10%, var(--find-bg)); + border: 1px solid color-mix(in srgb, var(--find-fg) 32%, var(--find-bg)); + border-radius: 4px; + box-shadow: 0 2px 10px rgba(0, 0, 0, 0.35); + pointer-events: none; + } + #link-tip .link-hint { + opacity: 0.7; + }
+ + + + diff --git a/src/qtxterm/assets/terminal.js b/src/qtxterm/assets/terminal.js index f96be6b..812ff80 100644 --- a/src/qtxterm/assets/terminal.js +++ b/src/qtxterm/assets/terminal.js @@ -14,10 +14,19 @@ ? requestedScrollback : 1000; - // Paint the page ground too, not just xterm's canvas - otherwise the - // margin around the grid stays default-white on dark themes. - const applyPageBackground = (background) => { - document.body.style.background = background; + const backgroundImage = params.get("backgroundImage") || ""; + const rawOpacity = parseInt(params.get("backgroundOpacity"), 10); + const backgroundOpacity = Number.isFinite(rawOpacity) ? rawOpacity : 30; + + // "#rrggbb" -> "rgba(r, g, b, a)", so the theme colour can be laid over an + // image at partial strength. Anything unparseable falls back to opaque + // black rather than producing invalid CSS, which would drop the whole + // declaration and show the image at full strength behind the text. + const withAlpha = (hex, alpha) => { + const match = /^#([0-9a-f]{2})([0-9a-f]{2})([0-9a-f]{2})$/i.exec(hex || ""); + if (!match) return `rgba(0, 0, 0, ${alpha})`; + const [r, g, b] = match.slice(1).map((part) => parseInt(part, 16)); + return `rgba(${r}, ${g}, ${b}, ${alpha})`; }; // xterm.js scrolls a plain div, so the bar is Chromium's own. Restyled @@ -45,21 +54,353 @@ `; }; - applyPageBackground(theme.background); + // Colors for the find bar and for the highlight xterm paints on each match. + // Both are derived from the terminal theme rather than fixed, so a find bar + // is never a light rectangle on a black terminal. + // + // *On*, not behind: xterm draws search decorations over the glyphs, so an + // opaque highlight erases the text it is pointing at. Measured - reusing + // the theme's `selectionBackground` looked like the safe choice and turned + // every match into a solid white block on VS Code Dark High Contrast, whose + // selection colour is #ffffff. The tint has to be translucent, and the + // alpha is the whole design: + // + // - yellow is the one hue every theme here keeps clear of its background + // and its foreground, so it marks a match without being mistaken for + // either + // - 0x40 (25%) for the crowd, 0x80 (50%) for the current match - enough + // separation to find the active one at a glance, still transparent + // enough to read the text through + // - the active match's border is the theme's *foreground*, which is + // contrasty against the background by definition, so the outline holds + // up in a theme nobody has tried yet + let findDecorations = {}; + + const applyFindTheme = (t) => { + const root = document.documentElement.style; + root.setProperty("--find-bg", t.background); + root.setProperty("--find-fg", t.foreground); + root.setProperty("--find-accent", t.blue); + root.setProperty("--find-error", t.red); + findDecorations = { + matchBackground: `${t.yellow}40`, + matchBorder: t.yellow, + activeMatchBackground: `${t.yellow}80`, + activeMatchBorder: t.foreground, + // Not optional, despite reading like it. The addon passes these + // straight to registerDecoration as overviewRulerOptions.color, and an + // undefined color throws from inside the search - which surfaced as + // "No results" on a query with three matches, because the throw + // happened while selecting the match it had already found. They are + // inert until the terminal is given an overviewRulerWidth, so this + // costs nothing but keeps the search from breaking. + matchOverviewRuler: t.yellow, + activeMatchColorOverviewRuler: t.foreground, + }; + }; + + // Paint the page ground too, not just xterm's canvas - otherwise the + // margin around the grid stays default-white on dark themes. + // + // With a background image the theme colour becomes a *veil* over it rather + // than the ground itself: the image is the bottom layer and a flat wash of + // the theme background sits on top at (100 - strength), which is what keeps + // text readable over a photograph. Done as a gradient layer rather than an + // extra element so it stays one CSS property and cannot fall out of sync + // with the terminal's own geometry. + // + // The image spans the whole *tab*, not each pane. Every pane is a separate + // page, so left alone each one paints the entire picture and a tab split + // three ways shows it three times. Instead Qt pushes each pane its own + // rectangle within the tab (applyBackgroundGeometry) and the page draws + // only its slice, so the panes reassemble into one continuous image. + let pageBackground = theme.background; + // Starts null rather than at the query-param value, so the first + // applyPageBackground call below counts as a change and measures the + // image. Seeding it with the real value made that call a no-op, the + // measurement never ran, and every pane silently fell back to painting the + // whole picture itself - the exact bug this spanning code exists to fix. + let pageImage = null; + let pageOpacity = backgroundOpacity; + // {x, y, width, height} of this pane inside its tab, in CSS pixels. + let paneRect = null; + // The image's intrinsic size, needed to reproduce `cover` by hand across + // the tab rather than per pane. Measured once per image. + let imageSize = null; + + const measureImage = (url, done) => { + if (!url) { + imageSize = null; + done(); + return; + } + const probe = new Image(); + probe.onload = () => { + imageSize = { width: probe.naturalWidth, height: probe.naturalHeight }; + done(); + }; + // A missing or unreadable file falls back to per-pane `cover` rather + // than leaving the background half-applied. + probe.onerror = () => { + imageSize = null; + done(); + }; + probe.src = url; + }; + + const paintBackground = () => { + const body = document.body.style; + body.backgroundColor = pageBackground; + if (!pageImage) { + body.backgroundImage = "none"; + return; + } + const veil = withAlpha(pageBackground, (100 - pageOpacity) / 100); + body.backgroundImage = `linear-gradient(${veil}, ${veil}), url("${pageImage}")`; + body.backgroundRepeat = "no-repeat"; + + const canSpan = + paneRect && imageSize && paneRect.width > 0 && paneRect.height > 0; + if (!canSpan) { + // Before the first geometry arrives, or without a measurable image. + body.backgroundSize = "auto, cover"; + body.backgroundPosition = "0 0, center"; + return; + } + // `cover` computed against the tab, then shifted by where this pane sits + // in it. Doing it by hand is the only way: CSS `cover` always resolves + // against the element's own box, which is exactly the per-pane repeat + // being avoided here. + const scale = Math.max( + paneRect.tabWidth / imageSize.width, + paneRect.tabHeight / imageSize.height, + ); + const drawWidth = imageSize.width * scale; + const drawHeight = imageSize.height * scale; + const left = (paneRect.tabWidth - drawWidth) / 2 - paneRect.x; + const top = (paneRect.tabHeight - drawHeight) / 2 - paneRect.y; + // Two layers: the veil covers this pane, the image is positioned across + // the tab. Their sizes and positions are per-layer and comma separated. + body.backgroundSize = `auto, ${drawWidth}px ${drawHeight}px`; + body.backgroundPosition = `0 0, ${left}px ${top}px`; + }; + + const applyPageBackground = (background, image, opacity) => { + const imageChanged = image !== pageImage; + pageBackground = background; + pageImage = image; + pageOpacity = opacity; + if (imageChanged) { + measureImage(image, paintBackground); + } else { + paintBackground(); + } + }; + + // Called from Python (TerminalWidget.set_background_geometry) whenever this + // pane's place in its tab changes - a split, a resize, a divider drag, a + // pane moved or closed. + window.applyBackgroundGeometry = function (x, y, tabWidth, tabHeight) { + paneRect = { x, y, tabWidth, tabHeight, width: tabWidth, height: tabHeight }; + paintBackground(); + }; + + applyPageBackground(theme.background, backgroundImage, backgroundOpacity); applyScrollbarTint(theme.foreground); + applyFindTheme(theme); + + // xterm gets a transparent background when an image is set, so the image + // and its veil show through; without an image it keeps the solid colour, + // so the no-image case renders exactly as it did before this existed. + const termTheme = (t, image) => + image ? { ...t, background: "#00000000" } : t; const term = new Terminal({ cursorBlink: true, + // Required by the search addon: the match highlights are drawn with + // registerMarker/registerDecoration, which xterm 5.5 still classes as + // proposed API and refuses to run without this. Without it every search + // throws *after* finding its matches, which reads as "No results" on a + // query that plainly matches. Safe here because xterm is vendored at a + // pinned version, so the proposed API cannot change underneath us - see + // xterm/VENDORED.md. + allowProposedApi: true, + // Lets the grid sit on the page's background instead of painting an + // opaque one of its own, which is what makes a background image + // visible at all. Set at construction because xterm reads it once. + allowTransparency: true, fontFamily, fontSize, scrollback, - theme, + theme: termTheme(theme, backgroundImage), }); const fitAddon = new FitAddon.FitAddon(); term.loadAddon(fitAddon); + const searchAddon = new SearchAddon.SearchAddon(); + term.loadAddon(searchAddon); term.open(document.getElementById("terminal")); fitAddon.fit(); + const findBar = document.getElementById("find-bar"); + const findInput = document.getElementById("find-input"); + const findCount = document.getElementById("find-count"); + const findCase = document.getElementById("find-case"); + const findRegex = document.getElementById("find-regex"); + + const setResultCount = (count, index) => { + if (!findInput.value) { + findCount.textContent = ""; + } else if (count > 0) { + findCount.textContent = `${index + 1} of ${count}`; + } else { + findCount.textContent = "No results"; + } + findBar.classList.toggle( + "no-results", + Boolean(findInput.value) && count === 0, + ); + }; + + searchAddon.onDidChangeResults((results) => { + setResultCount(results ? results.resultCount : 0, results ? results.resultIndex : -1); + }); + + const findOptions = (incremental) => ({ + caseSensitive: findCase.classList.contains("active"), + regex: findRegex.classList.contains("active"), + decorations: findDecorations, + incremental, + }); + + // `incremental` keeps the current match anchored while the query grows, so + // typing "err" doesn't walk three matches on the way to the word you meant. + // It only applies to typing - the next/previous buttons must actually move. + const runFind = (backwards, incremental) => { + if (!findInput.value) { + searchAddon.clearDecorations(); + setResultCount(0, -1); + return; + } + const options = findOptions(incremental); + try { + if (backwards) { + searchAddon.findPrevious(findInput.value, options); + } else { + searchAddon.findNext(findInput.value, options); + } + } catch (e) { + console.error("find failed", e); + // A half-typed regex ("[a" ) throws rather than simply not matching. + // Reported as no results, which is what it is, instead of leaving the + // count stale on a query that found nothing. + setResultCount(0, -1); + } + }; + + findInput.addEventListener("input", () => runFind(false, true)); + findInput.addEventListener("keydown", (event) => { + if (event.key === "Enter") { + event.preventDefault(); + runFind(event.shiftKey, false); + } else if (event.key === "Escape") { + event.preventDefault(); + window.hideFind(); + } + }); + + const bindToggle = (button) => { + button.addEventListener("click", () => { + button.classList.toggle("active"); + button.setAttribute( + "aria-pressed", + String(button.classList.contains("active")), + ); + // Re-run from where we are rather than from the top of the buffer. + // The active match can still step forward by one - the addon resumes + // from the current selection - but you stay in the same region of the + // scrollback instead of being thrown back to the first match. + runFind(false, true); + findInput.focus(); + }); + }; + bindToggle(findCase); + bindToggle(findRegex); + + const bindStep = (button, backwards) => { + button.addEventListener("click", () => { + runFind(backwards, false); + findInput.focus(); + }); + }; + bindStep(document.getElementById("find-prev"), true); + bindStep(document.getElementById("find-next"), false); + document.getElementById("find-close").addEventListener("click", () => { + window.hideFind(); + }); + + // Called from Python (TerminalWidget.show_find / hide_find). + window.showFind = function () { + findBar.hidden = false; + findInput.focus(); + // Selected rather than cleared, so the previous query is still there to + // step through with Enter but is replaced by whatever you type next. + findInput.select(); + if (findInput.value) runFind(false, true); + }; + + window.hideFind = function () { + findBar.hidden = true; + searchAddon.clearDecorations(); + // Focus has to go back explicitly: it is sitting in the find input, so + // without this the terminal is visible but not typeable. + term.focus(); + }; + + window.isFindOpen = function () { + return !findBar.hidden; + }; + + const linkTip = document.getElementById("link-tip"); + const isMac = /Mac|iPhone|iPad/.test(navigator.platform || ""); + + const showLinkTip = (event, uri) => { + linkTip.textContent = uri; + const hint = document.createElement("div"); + hint.className = "link-hint"; + hint.textContent = `${isMac ? "Cmd" : "Ctrl"}+click to open`; + linkTip.appendChild(hint); + linkTip.hidden = false; + + // Placed after unhiding, because a hidden element measures 0x0 and the + // tip would be flipped by the edge checks below on every first hover. + const pad = 12; + const box = linkTip.getBoundingClientRect(); + let left = event.clientX + pad; + let top = event.clientY + pad; + // Kept inside the viewport: a link near the right edge is exactly the + // one whose full URL you wanted to read. + if (left + box.width > window.innerWidth) { + left = Math.max(0, event.clientX - box.width - pad); + } + if (top + box.height > window.innerHeight) { + top = Math.max(0, event.clientY - box.height - pad); + } + linkTip.style.left = `${left}px`; + linkTip.style.top = `${top}px`; + }; + + const hideLinkTip = () => { + linkTip.hidden = true; + }; + + // Called from Python (TerminalWidget.focus_pane). Focusing the + // QWebEngineView alone leaves the keyboard on the page rather than in the + // terminal - xterm reads keystrokes from a hidden textarea, and only this + // puts the caret there. + window.focusTerminal = function () { + term.focus(); + }; + // Called from Python (TerminalWidget.paste). Routed through term.paste() // rather than written straight to the PTY so bracketed paste mode is // honored - editors and shells that enable it need the text wrapped, or a @@ -71,14 +412,23 @@ // Called from Python (TerminalWidget.apply_appearance) to live-update an // already-open tab without reloading the page. window.applyAppearance = function (options) { - term.options.theme = options.theme; + const image = options.backgroundImage || ""; + term.options.theme = termTheme(options.theme, image); term.options.fontFamily = options.fontFamily; term.options.fontSize = options.fontSize; // Lowering this drops the oldest lines immediately, which is the point: // a terminal left open for days is holding every one of them. term.options.scrollback = options.scrollback; - applyPageBackground(options.theme.background); + applyPageBackground( + options.theme.background, + image, + options.backgroundOpacity, + ); applyScrollbarTint(options.theme.foreground); + applyFindTheme(options.theme); + // Decorations already on screen were painted in the old theme's colors, + // so an open find has to be re-run to pick the new ones up. + if (!findBar.hidden) runFind(false, true); fitAddon.fit(); // A different font size means a different number of cells in the same // pixels, and the shell has to be told or it wraps to the old width. @@ -121,6 +471,29 @@ } }; + // Clickable URLs. Loaded here rather than beside the other addons + // because opening one means calling Python, and the bridge only exists + // inside this callback. + // + // Ctrl+click (Cmd on macOS), not a plain click, following VS Code's + // terminal, Windows Terminal and iTerm2. A bare click has a job already - + // placing the cursor and starting a selection - and terminal output is + // full of URLs you did not mean to visit. The handler is still called on + // an unmodified click, and returning without acting leaves the click to + // do its normal thing. + const openLink = (event, uri) => { + if (!event.ctrlKey && !event.metaKey) return; + hideLinkTip(); + bridge.openLink(uri); + }; + + term.loadAddon( + new WebLinksAddon.WebLinksAddon(openLink, { + hover: (event, uri) => showLinkTip(event, uri), + leave: () => hideLinkTip(), + }), + ); + bridge.loaded(); }); })(); diff --git a/src/qtxterm/assets/xterm/VENDORED.md b/src/qtxterm/assets/xterm/VENDORED.md index 7ddba19..a57317b 100644 --- a/src/qtxterm/assets/xterm/VENDORED.md +++ b/src/qtxterm/assets/xterm/VENDORED.md @@ -5,6 +5,8 @@ | `xterm.js` | https://unpkg.com/@xterm/xterm@5.5.0/lib/xterm.js | 5.5.0 | | `xterm.css` | https://unpkg.com/@xterm/xterm@5.5.0/css/xterm.css | 5.5.0 | | `addon-fit.js` | https://unpkg.com/@xterm/addon-fit@0.10.0/lib/addon-fit.js | 0.10.0 | +| `addon-search.js` | https://unpkg.com/@xterm/addon-search@0.15.0/lib/addon-search.js | 0.15.0 | +| `addon-web-links.js` | https://unpkg.com/@xterm/addon-web-links@0.11.0/lib/addon-web-links.js | 0.11.0 | `qwebchannel.js` is NOT vendored here — Qt auto-registers it as a resource (`qrc:///qtwebchannel/qwebchannel.js`) whenever `QtWebChannel` is imported diff --git a/src/qtxterm/assets/xterm/addon-search.js b/src/qtxterm/assets/xterm/addon-search.js new file mode 100644 index 0000000..e70b0fb --- /dev/null +++ b/src/qtxterm/assets/xterm/addon-search.js @@ -0,0 +1,2 @@ +!function(e,t){"object"==typeof exports&&"object"==typeof module?module.exports=t():"function"==typeof define&&define.amd?define([],t):"object"==typeof exports?exports.SearchAddon=t():e.SearchAddon=t()}(self,(()=>(()=>{"use strict";var e={345:(e,t)=>{Object.defineProperty(t,"__esModule",{value:!0}),t.runAndSubscribe=t.forwardEvent=t.EventEmitter=void 0,t.EventEmitter=class{constructor(){this._listeners=[],this._disposed=!1}get event(){return this._event||(this._event=e=>(this._listeners.push(e),{dispose:()=>{if(!this._disposed)for(let t=0;tt.fire(e)))},t.runAndSubscribe=function(e,t){return t(void 0),e((e=>t(e)))}},859:(e,t)=>{function i(e){for(const t of e)t.dispose();e.length=0}Object.defineProperty(t,"__esModule",{value:!0}),t.getDisposeArrayDisposable=t.disposeArray=t.toDisposable=t.MutableDisposable=t.Disposable=void 0,t.Disposable=class{constructor(){this._disposables=[],this._isDisposed=!1}dispose(){this._isDisposed=!0;for(const e of this._disposables)e.dispose();this._disposables.length=0}register(e){return this._disposables.push(e),e}unregister(e){const t=this._disposables.indexOf(e);-1!==t&&this._disposables.splice(t,1)}},t.MutableDisposable=class{constructor(){this._isDisposed=!1}get value(){return this._isDisposed?void 0:this._value}set value(e){this._isDisposed||e===this._value||(this._value?.dispose(),this._value=e)}clear(){this.value=void 0}dispose(){this._isDisposed=!0,this._value?.dispose(),this._value=void 0}},t.toDisposable=function(e){return{dispose:e}},t.disposeArray=i,t.getDisposeArrayDisposable=function(e){return{dispose:()=>i(e)}}}},t={};function i(s){var r=t[s];if(void 0!==r)return r.exports;var o=t[s]={exports:{}};return e[s](o,o.exports,i),o.exports}var s={};return(()=>{var e=s;Object.defineProperty(e,"__esModule",{value:!0}),e.SearchAddon=void 0;const t=i(345),r=i(859),o=" ~!@#$%^&*()+`-=[]{}|\\;:\"',./<>?";class n extends r.Disposable{constructor(e){super(),this._highlightedLines=new Set,this._highlightDecorations=[],this._selectedDecoration=this.register(new r.MutableDisposable),this._linesCacheTimeoutId=0,this._linesCacheDisposables=new r.MutableDisposable,this._onDidChangeResults=this.register(new t.EventEmitter),this.onDidChangeResults=this._onDidChangeResults.event,this._highlightLimit=e?.highlightLimit??1e3}activate(e){this._terminal=e,this.register(this._terminal.onWriteParsed((()=>this._updateMatches()))),this.register(this._terminal.onResize((()=>this._updateMatches()))),this.register((0,r.toDisposable)((()=>this.clearDecorations())))}_updateMatches(){this._highlightTimeout&&window.clearTimeout(this._highlightTimeout),this._cachedSearchTerm&&this._lastSearchOptions?.decorations&&(this._highlightTimeout=setTimeout((()=>{const e=this._cachedSearchTerm;this._cachedSearchTerm=void 0,this.findPrevious(e,{...this._lastSearchOptions,incremental:!0,noScroll:!0})}),200))}clearDecorations(e){this._selectedDecoration.clear(),(0,r.disposeArray)(this._highlightDecorations),this._highlightDecorations=[],this._highlightedLines.clear(),e||(this._cachedSearchTerm=void 0)}clearActiveDecoration(){this._selectedDecoration.clear()}findNext(e,t){if(!this._terminal)throw new Error("Cannot use addon until it has been loaded");const i=!this._lastSearchOptions||this._didOptionsChange(this._lastSearchOptions,t);this._lastSearchOptions=t,t?.decorations&&(void 0===this._cachedSearchTerm||e!==this._cachedSearchTerm||i)&&this._highlightAllMatches(e,t);const s=this._findNextAndSelect(e,t);return this._fireResults(t),this._cachedSearchTerm=e,s}_highlightAllMatches(e,t){if(!this._terminal)throw new Error("Cannot use addon until it has been loaded");if(!e||0===e.length)return void this.clearDecorations();t=t||{},this.clearDecorations(!0);const i=[];let s,r=this._find(e,0,0,t);for(;r&&(s?.row!==r.row||s?.col!==r.col)&&!(i.length>=this._highlightLimit);)s=r,i.push(s),r=this._find(e,s.col+s.term.length>=this._terminal.cols?s.row+1:s.row,s.col+s.term.length>=this._terminal.cols?0:s.col+1,t);for(const e of i){const i=this._createResultDecoration(e,t.decorations);i&&(this._highlightedLines.add(i.marker.line),this._highlightDecorations.push({decoration:i,match:e,dispose(){i.dispose()}}))}}_find(e,t,i,s){if(!this._terminal||!e||0===e.length)return this._terminal?.clearSelection(),void this.clearDecorations();if(i>this._terminal.cols)throw new Error(`Invalid col: ${i} to search in terminal of ${this._terminal.cols} cols`);let r;this._initLinesCache();const o={startRow:t,startCol:i};if(r=this._findInLine(e,o,s),!r)for(let i=t+1;i=0&&(n.startRow=i,h=this._findInLine(e,n,t,o),!h);i--);}if(!h&&s!==this._terminal.buffer.active.baseY+this._terminal.rows-1)for(let i=this._terminal.buffer.active.baseY+this._terminal.rows-1;i>=s&&(n.startRow=i,h=this._findInLine(e,n,t,o),!h);i--);return this._selectResult(h,t?.decorations,t?.noScroll)}_initLinesCache(){const e=this._terminal;this._linesCache||(this._linesCache=new Array(e.buffer.active.length),this._linesCacheDisposables.value=(0,r.getDisposeArrayDisposable)([e.onLineFeed((()=>this._destroyLinesCache())),e.onCursorMove((()=>this._destroyLinesCache())),e.onResize((()=>this._destroyLinesCache()))])),window.clearTimeout(this._linesCacheTimeoutId),this._linesCacheTimeoutId=window.setTimeout((()=>this._destroyLinesCache()),15e3)}_destroyLinesCache(){this._linesCache=void 0,this._linesCacheDisposables.clear(),this._linesCacheTimeoutId&&(window.clearTimeout(this._linesCacheTimeoutId),this._linesCacheTimeoutId=0)}_isWholeWord(e,t,i){return(0===e||o.includes(t[e-1]))&&(e+i.length===t.length||o.includes(t[e+i.length]))}_findInLine(e,t,i={},s=!1){const r=this._terminal,o=t.startRow,n=t.startCol,h=r.buffer.active.getLine(o);if(h?.isWrapped)return s?void(t.startCol+=r.cols):(t.startRow--,t.startCol+=r.cols,this._findInLine(e,t,i));let a=this._linesCache?.[o];a||(a=this._translateBufferLineToStringWithWrap(o,!0),this._linesCache&&(this._linesCache[o]=a));const[l,c]=a,d=this._bufferColsToStringOffset(o,n),_=i.caseSensitive?e:e.toLowerCase(),u=i.caseSensitive?l:l.toLowerCase();let f=-1;if(i.regex){const t=RegExp(_,"g");let i;if(s)for(;i=t.exec(u.slice(0,d));)f=t.lastIndex-i[0].length,e=i[0],t.lastIndex-=e.length-1;else i=t.exec(u.slice(d)),i&&i[0].length>0&&(f=d+(t.lastIndex-i[0].length),e=i[0])}else s?d-_.length>=0&&(f=u.lastIndexOf(_,d-_.length)):f=u.indexOf(_,d);if(f>=0){if(i.wholeWord&&!this._isWholeWord(f,u,e))return;let t=0;for(;t=c[t+1];)t++;let s=t;for(;s=c[s+1];)s++;const n=f-c[t],h=f+e.length-c[s],a=this._stringLengthToBufferSize(o+t,n);return{term:e,col:a,row:o+t,size:this._stringLengthToBufferSize(o+s,h)-a+r.cols*(s-t)}}}_stringLengthToBufferSize(e,t){const i=this._terminal.buffer.active.getLine(e);if(!i)return 0;for(let e=0;e1&&(t-=r.length-1);const o=i.getCell(e+1);o&&0===o.getWidth()&&t++}return t}_bufferColsToStringOffset(e,t){const i=this._terminal;let s=e,r=0,o=i.buffer.active.getLine(s);for(;t>0&&o;){for(let e=0;ethis._applyStyles(e,t.activeMatchBorder,!0)))),s.push(o.onDispose((()=>(0,r.disposeArray)(s)))),this._selectedDecoration.value={decoration:o,match:e,dispose(){o.dispose()}}}}}if(!i&&(e.row>=s.buffer.active.viewportY+s.rows||e.rowthis._applyStyles(e,t.matchBorder,!1)))),e.push(o.onDispose((()=>(0,r.disposeArray)(e))))}return o}}e.SearchAddon=n})(),s})())); +//# sourceMappingURL=addon-search.js.map \ No newline at end of file diff --git a/src/qtxterm/assets/xterm/addon-web-links.js b/src/qtxterm/assets/xterm/addon-web-links.js new file mode 100644 index 0000000..2131376 --- /dev/null +++ b/src/qtxterm/assets/xterm/addon-web-links.js @@ -0,0 +1,2 @@ +!function(e,t){"object"==typeof exports&&"object"==typeof module?module.exports=t():"function"==typeof define&&define.amd?define([],t):"object"==typeof exports?exports.WebLinksAddon=t():e.WebLinksAddon=t()}(self,(()=>(()=>{"use strict";var e={6:(e,t)=>{function n(e){try{const t=new URL(e),n=t.password&&t.username?`${t.protocol}//${t.username}:${t.password}@${t.host}`:t.username?`${t.protocol}//${t.username}@${t.host}`:`${t.protocol}//${t.host}`;return e.toLocaleLowerCase().startsWith(n.toLocaleLowerCase())}catch(e){return!1}}Object.defineProperty(t,"__esModule",{value:!0}),t.LinkComputer=t.WebLinkProvider=void 0,t.WebLinkProvider=class{constructor(e,t,n,o={}){this._terminal=e,this._regex=t,this._handler=n,this._options=o}provideLinks(e,t){const n=o.computeLink(e,this._regex,this._terminal,this._handler);t(this._addCallbacks(n))}_addCallbacks(e){return e.map((e=>(e.leave=this._options.leave,e.hover=(t,n)=>{if(this._options.hover){const{range:o}=e;this._options.hover(t,n,o)}},e)))}};class o{static computeLink(e,t,r,i){const s=new RegExp(t.source,(t.flags||"")+"g"),[a,c]=o._getWindowedLineStrings(e-1,r),l=a.join("");let d;const p=[];for(;d=s.exec(l);){const e=d[0];if(!n(e))continue;const[t,s]=o._mapStrIdx(r,c,0,d.index),[a,l]=o._mapStrIdx(r,t,s,e.length);if(-1===t||-1===s||-1===a||-1===l)continue;const h={start:{x:s+1,y:t+1},end:{x:l,y:a+1}};p.push({range:h,text:e,activate:i})}return p}static _getWindowedLineStrings(e,t){let n,o=e,r=e,i=0,s="";const a=[];if(n=t.buffer.active.getLine(e)){const e=n.translateToString(!0);if(n.isWrapped&&" "!==e[0]){for(i=0;(n=t.buffer.active.getLine(--o))&&i<2048&&(s=n.translateToString(!0),i+=s.length,a.push(s),n.isWrapped&&-1===s.indexOf(" ")););a.reverse()}for(a.push(e),i=0;(n=t.buffer.active.getLine(++r))&&n.isWrapped&&i<2048&&(s=n.translateToString(!0),i+=s.length,a.push(s),-1===s.indexOf(" ")););}return[a,o]}static _mapStrIdx(e,t,n,o){const r=e.buffer.active,i=r.getNullCell();let s=n;for(;o;){const e=r.getLine(t);if(!e)return[-1,-1];for(let n=s;n{var e=o;Object.defineProperty(e,"__esModule",{value:!0}),e.WebLinksAddon=void 0;const t=n(6),r=/(https?|HTTPS?):[/]{2}[^\s"'!*(){}|\\\^<>`]*[^\s"':,.!?{}|\\\^~\[\]`()<>]/;function i(e,t){const n=window.open();if(n){try{n.opener=null}catch{}n.location.href=t}else console.warn("Opening link blocked as opener could not be cleared")}e.WebLinksAddon=class{constructor(e=i,t={}){this._handler=e,this._options=t}activate(e){this._terminal=e;const n=this._options,o=n.urlRegex||r;this._linkProvider=this._terminal.registerLinkProvider(new t.WebLinkProvider(this._terminal,o,this._handler,n))}dispose(){this._linkProvider?.dispose()}}})(),o})())); +//# sourceMappingURL=addon-web-links.js.map \ No newline at end of file diff --git a/src/qtxterm/browser_widget.py b/src/qtxterm/browser_widget.py index 5846dd1..38cbc1f 100644 --- a/src/qtxterm/browser_widget.py +++ b/src/qtxterm/browser_widget.py @@ -114,6 +114,12 @@ def _on_url_changed(self, url: QUrl) -> None: self._address.setText(url.toString()) self.host_changed.emit(url.host() or self.default_title) + def focus_pane(self) -> None: + """Focus the page, not the address bar - this is a pane being + navigated to, and landing in the URL box would eat the next + keystrokes.""" + self._view.setFocus() + def shutdown(self) -> None: """Stop loading and release the page, mirroring TerminalWidget. diff --git a/src/qtxterm/exit_prefs.py b/src/qtxterm/exit_prefs.py new file mode 100644 index 0000000..a392bb5 --- /dev/null +++ b/src/qtxterm/exit_prefs.py @@ -0,0 +1,68 @@ +"""When a pane should close itself after its shell exits. + +Before this, an exited shell left a dead pane showing "[process exited with +code 0]" until you closed it by hand, which every other terminal treats as +the exception rather than the rule. + +Three settings rather than a checkbox, because the interesting case is the +middle one. A shell you exited on purpose (`exit`, Ctrl+D) should take its +pane with it; a shell that *died* has usually printed why, and closing the +pane throws that away just as you needed to read it. Matching Windows +Terminal's closeOnExit, which arrived at the same three. +""" + +from __future__ import annotations + +from PySide6.QtCore import QObject, QSettings, Signal + +_KEY = "session/closeOnExit" + +# Leave the pane open whatever happened - what qtxterm did before this +# existed, kept so nobody's habit is broken by an upgrade. +CLOSE_NEVER = "never" +# Close only on a zero exit code, so a crash leaves its error on screen. +CLOSE_CLEAN = "clean" +# Always close, even when the shell died. For people who keep their errors +# somewhere other than the terminal. +CLOSE_ALWAYS = "always" + +CHOICES = (CLOSE_CLEAN, CLOSE_ALWAYS, CLOSE_NEVER) +LABELS = { + CLOSE_CLEAN: "Close it, unless the shell failed", + CLOSE_ALWAYS: "Always close it", + CLOSE_NEVER: "Leave it open", +} +DEFAULT = CLOSE_CLEAN + + +def should_close(choice: str, exit_code: int) -> bool: + """Whether a pane whose shell exited with `exit_code` should close.""" + if choice == CLOSE_ALWAYS: + return True + if choice == CLOSE_CLEAN: + return exit_code == 0 + return False + + +class PaneExitStore(QObject): + """Loads/saves the close-on-exit choice, the same shape as the other stores.""" + + changed = Signal() + + def __init__(self, settings: QSettings) -> None: + super().__init__() + self._settings = settings + self.current = self._load() + + def _load(self) -> str: + # Anything unrecognised falls back rather than raising: this file is + # hand-editable, and a typo should not stop the app starting. + choice = self._settings.value(_KEY, DEFAULT) + return choice if choice in CHOICES else DEFAULT + + def save(self, choice: str) -> None: + if choice not in CHOICES: + raise ValueError(f"unknown close-on-exit choice: {choice!r}") + self.current = choice + self._settings.setValue(_KEY, choice) + self.changed.emit() diff --git a/src/qtxterm/keybindings.py b/src/qtxterm/keybindings.py new file mode 100644 index 0000000..adda550 --- /dev/null +++ b/src/qtxterm/keybindings.py @@ -0,0 +1,181 @@ +"""User overrides for the keyboard shortcuts. + +Every terminal worth using lets you rebind its keys, and the reason is not +taste: a shortcut can be taken away by something the app cannot see. A tiling +window manager that owns Alt+Arrow, a desktop that claims Ctrl+Alt+Left, a +shell binding somebody relies on - none of these are visible from inside +qtxterm, and no default table can be right for all of them. Rebinding is the +escape hatch that makes the defaults a starting point rather than a verdict. + +Two decisions shape the file this writes. + +**Only the differences are stored.** Saving the whole resolved table would +freeze today's defaults into every config file: a later version that improves +a binding, or adds an action, would never reach anyone who had opened the +editor once. Storing just the overrides means an untouched action keeps +following the defaults forever. + +**A conflict is refused, not accepted.** Two actions sharing a sequence makes +Qt fire *neither* - it reports the ambiguity and gives up - so a last-one-wins +policy would quietly disable both. The store rejects the save and names the +action already holding the chord. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import platformdirs +from PySide6.QtCore import QObject, Signal +from PySide6.QtGui import QKeySequence + +from qtxterm import shortcuts + +# Bumped only if the on-disk shape changes in a way a reader must know about. +FORMAT_VERSION = 1 + + +class ConflictError(ValueError): + """A sequence is already bound to a different action.""" + + def __init__(self, sequence: str, action: str) -> None: + super().__init__( + f"{sequence} is already bound to {shortcuts.label_for(action)}" + ) + self.sequence = sequence + self.action = action + + +def default_keybindings_path() -> Path: + return ( + Path(platformdirs.user_config_dir("qtxterm", appauthor=False)) + / "keybindings.json" + ) + + +def normalise(sequence: str) -> str: + """Qt's canonical spelling of a sequence, or "" if it isn't one. + + Round-tripping through QKeySequence means "ctrl+shift+t" and "Ctrl+Shift+T" + are stored identically, so a hand-edited file cannot produce a binding that + looks set in the editor and never matches a key. + """ + parsed = QKeySequence(sequence) + return "" if parsed.isEmpty() else parsed.toString() + + +class KeybindingStore(QObject): + """Resolves an action to its key sequences, honouring user overrides. + + Emits `changed` after every save, so the widget that owns the QShortcuts + can rebuild them - the same pattern PresetStore uses for menus. + """ + + changed = Signal() + + def __init__(self, path: Path | None = None) -> None: + super().__init__() + self._path = path or default_keybindings_path() + self._overrides: dict[str, list[str]] = self._load() + + def _load(self) -> dict[str, list[str]]: + try: + raw = json.loads(self._path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + # A missing file is the normal case on first run, and a corrupt one + # should not stop the app starting - the cost of ignoring it is + # falling back to defaults, which are always usable. + return {} + bindings = raw.get("bindings") if isinstance(raw, dict) else None + if not isinstance(bindings, dict): + return {} + + cleaned: dict[str, list[str]] = {} + known = set(shortcuts.all_actions()) + for action, sequences in bindings.items(): + # Unknown actions are dropped rather than kept: they are either a + # typo or a binding from a newer version, and carrying them would + # let them collide with something real later. + if action not in known or not isinstance(sequences, list): + continue + valid = [normalise(s) for s in sequences if isinstance(s, str)] + cleaned[action] = [s for s in valid if s] + return cleaned + + def _save(self) -> None: + self._path.parent.mkdir(parents=True, exist_ok=True) + payload = {"version": FORMAT_VERSION, "bindings": self._overrides} + self._path.write_text(json.dumps(payload, indent=2), encoding="utf-8") + self.changed.emit() + + def sequences_for(self, action: str) -> list[str]: + """The sequences in force for `action` - the override, or the default.""" + if action in self._overrides: + return list(self._overrides[action]) + return shortcuts.sequences_for(action) + + def display_sequences_for(self, action: str) -> list[str]: + """As above, named the way this platform's users see them.""" + return [shortcuts.display_sequence(s) for s in self.sequences_for(action)] + + def is_customised(self, action: str) -> bool: + return action in self._overrides + + def holder_of(self, sequence: str, ignoring: str | None = None) -> str | None: + """Which action currently owns `sequence`, if any.""" + wanted = normalise(sequence) + if not wanted: + return None + for action in shortcuts.all_actions(): + if action == ignoring: + continue + if wanted in self.sequences_for(action): + return action + return None + + def set_sequences(self, action: str, sequences: list[str]) -> None: + """Rebind `action`, refusing anything another action already holds. + + An empty list is allowed and means "no shortcut": an action nobody + wants a key for is a legitimate thing to ask for, and is different + from resetting to the default. + """ + if action not in set(shortcuts.all_actions()): + raise KeyError(action) + + cleaned: list[str] = [] + for sequence in sequences: + normalised = normalise(sequence) + if not normalised: + raise ValueError(f"not a key sequence: {sequence!r}") + holder = self.holder_of(normalised, ignoring=action) + if holder is not None: + raise ConflictError(normalised, holder) + if normalised not in cleaned: + cleaned.append(normalised) + + self._overrides[action] = cleaned + self._save() + + def reset(self, action: str) -> None: + """Drop the override so `action` follows the defaults again.""" + if self._overrides.pop(action, None) is not None: + self._save() + + def reset_all(self) -> None: + if self._overrides: + self._overrides = {} + self._save() + + def conflicts(self) -> dict[str, list[str]]: + """Sequences held by more than one action, after overrides. + + The same check shortcuts.conflicts() makes over the defaults, repeated + here because an override can introduce a collision the table never had. + """ + seen: dict[str, list[str]] = {} + for action in shortcuts.all_actions(): + for sequence in self.sequences_for(action): + seen.setdefault(sequence, []).append(action) + return {seq: actions for seq, actions in seen.items() if len(actions) > 1} diff --git a/src/qtxterm/keybindings_dialog.py b/src/qtxterm/keybindings_dialog.py new file mode 100644 index 0000000..4165b5b --- /dev/null +++ b/src/qtxterm/keybindings_dialog.py @@ -0,0 +1,216 @@ +"""Editor for the keyboard shortcuts. + +Its own dialog rather than another row in Preferences: there are two dozen +actions, and Preferences is already a long form. + +The tree is two levels - an action, and under it each chord bound to it - +because several actions genuinely have more than one, and a flat "Ctrl+Shift+C, +Ctrl+Ins" cell gives nothing to click when you want to drop just one of them. +""" + +from __future__ import annotations + +from PySide6.QtCore import Qt +from PySide6.QtGui import QFont, QKeySequence +from PySide6.QtWidgets import ( + QDialog, + QDialogButtonBox, + QHBoxLayout, + QKeySequenceEdit, + QLabel, + QMessageBox, + QPushButton, + QTreeWidget, + QTreeWidgetItem, + QVBoxLayout, + QWidget, +) + +from qtxterm import shortcuts +from qtxterm.keybindings import ConflictError, KeybindingStore + +# Which level of the tree an item sits at. Stored rather than inferred from +# parent(), so a chord row still knows its action after the tree is rebuilt. +_ACTION_ROLE = Qt.ItemDataRole.UserRole +_IS_CHORD_ROLE = Qt.ItemDataRole.UserRole + 1 + + +class KeybindingsDialog(QDialog): + """Rebind, add, remove and reset keyboard shortcuts.""" + + def __init__(self, store: KeybindingStore, parent: QWidget | None = None) -> None: + super().__init__(parent) + self._store = store + self.setWindowTitle("Keyboard Shortcuts") + self.resize(600, 560) + + layout = QVBoxLayout(self) + + self._tree = QTreeWidget() + self._tree.setColumnCount(2) + self._tree.setHeaderLabels(["Action", "Shortcut"]) + self._tree.setRootIsDecorated(True) + self._tree.currentItemChanged.connect(lambda *_: self._refresh_buttons()) + layout.addWidget(self._tree, 1) + + hint = QLabel( + "Pick an action, press the keys you want, then Add. " + "Changed actions are shown in bold." + ) + hint.setWordWrap(True) + layout.addWidget(hint) + + entry = QHBoxLayout() + self._capture = QKeySequenceEdit() + # One chord, not a sequence of them: Qt will happily record up to four + # in a row ("Ctrl+K, Ctrl+S") and nothing in this app dispatches those. + self._capture.setMaximumSequenceLength(1) + self._capture.keySequenceChanged.connect(lambda *_: self._refresh_buttons()) + entry.addWidget(self._capture, 1) + + self._add_button = QPushButton("Add") + self._add_button.clicked.connect(self._add) + entry.addWidget(self._add_button) + + self._remove_button = QPushButton("Remove") + self._remove_button.clicked.connect(self._remove) + entry.addWidget(self._remove_button) + + self._reset_button = QPushButton("Reset") + self._reset_button.clicked.connect(self._reset) + entry.addWidget(self._reset_button) + layout.addLayout(entry) + + buttons = QDialogButtonBox(QDialogButtonBox.StandardButton.Close, parent=self) + reset_all = buttons.addButton( + "Reset All", QDialogButtonBox.ButtonRole.ResetRole + ) + reset_all.clicked.connect(self._reset_all) + buttons.rejected.connect(self.reject) + layout.addWidget(buttons) + + self._reload() + + # -- building --------------------------------------------------------- + + def _reload(self) -> None: + """Rebuild the tree, keeping the selected action where possible.""" + selected = self.current_action() + self._tree.clear() + for action in shortcuts.all_actions(): + chords = self._store.display_sequences_for(action) + # The action's row carries all of its chords, so the list reads as + # one line per action rather than two. The children exist only to + # give a single chord something to click when removing one of + # several, and stay collapsed until then. + item = QTreeWidgetItem( + [shortcuts.label_for(action), ", ".join(chords) or "None"] + ) + item.setData(0, _ACTION_ROLE, action) + item.setData(0, _IS_CHORD_ROLE, False) + if self._store.is_customised(action): + font = QFont(item.font(0)) + font.setBold(True) + item.setFont(0, font) + # One chord needs no expanding: Reset already covers it, and a + # disclosure arrow that reveals a copy of the row above is noise. + if len(chords) > 1: + for shown in chords: + chord = QTreeWidgetItem(["", shown]) + chord.setData(0, _ACTION_ROLE, action) + chord.setData(0, _IS_CHORD_ROLE, True) + item.addChild(chord) + self._tree.addTopLevelItem(item) + if action == selected: + self._tree.setCurrentItem(item) + self._tree.resizeColumnToContents(0) + self._refresh_buttons() + + def current_action(self) -> str | None: + item = self._tree.currentItem() + return item.data(0, _ACTION_ROLE) if item is not None else None + + def _current_is_chord(self) -> bool: + item = self._tree.currentItem() + return bool(item is not None and item.data(0, _IS_CHORD_ROLE)) + + def _refresh_buttons(self) -> None: + action = self.current_action() + self._add_button.setEnabled( + action is not None and not self._capture.keySequence().isEmpty() + ) + self._remove_button.setEnabled( + self._current_is_chord() + or bool(action and len(self._store.sequences_for(action)) == 1) + ) + self._reset_button.setEnabled( + action is not None and self._store.is_customised(action) + ) + + # -- editing ---------------------------------------------------------- + + def _add(self) -> None: + action = self.current_action() + sequence = self._capture.keySequence() + if action is None or sequence.isEmpty(): + return + # PortableText, not the native rendering: the store speaks Qt's + # spelling, and on macOS toString() would otherwise hand it the + # Command symbol rather than "Ctrl". + wanted = sequence.toString(QKeySequence.SequenceFormat.PortableText) + self._apply(action, [*self._store.sequences_for(action), wanted]) + + def _remove(self) -> None: + """Drop one chord: the selected one, or the only one there is.""" + item = self._tree.currentItem() + if item is None: + return + action = item.data(0, _ACTION_ROLE) + if not item.data(0, _IS_CHORD_ROLE): + # An action row with a single chord has no children to select, + # so Remove acts on that chord directly. + chords = self._store.display_sequences_for(action) + if len(chords) != 1: + return + shown = chords[0] + else: + shown = item.text(1) + remaining = [ + sequence + for sequence in self._store.sequences_for(action) + if shortcuts.display_sequence(sequence) != shown + ] + self._apply(action, remaining) + + def _reset(self) -> None: + action = self.current_action() + if action is not None: + self._store.reset(action) + self._reload() + + def _reset_all(self) -> None: + confirmed = QMessageBox.question( + self, + "Reset all shortcuts", + "Put every shortcut back to its default?", + QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, + QMessageBox.StandardButton.No, + ) + if confirmed == QMessageBox.StandardButton.Yes: + self._store.reset_all() + self._reload() + + def _apply(self, action: str, sequences: list[str]) -> None: + try: + self._store.set_sequences(action, sequences) + except ConflictError as clash: + # Named rather than silently overridden, because Qt fires neither + # of two shortcuts sharing a chord - so "it just stopped working" + # would be the alternative, for both actions. + QMessageBox.warning(self, "Shortcut already in use", str(clash)) + return + except ValueError as invalid: + QMessageBox.warning(self, "Not a shortcut", str(invalid)) + return + self._capture.clear() + self._reload() diff --git a/src/qtxterm/main_window.py b/src/qtxterm/main_window.py index 8b0cb75..7b1a677 100644 --- a/src/qtxterm/main_window.py +++ b/src/qtxterm/main_window.py @@ -9,7 +9,10 @@ from qtxterm.cron import CronStore from qtxterm.cron_menu import CronMenu from qtxterm.cron_scheduler import CronScheduler +from qtxterm.exit_prefs import PaneExitStore from qtxterm.help_dialog import HelpDialog +from qtxterm.keybindings import KeybindingStore +from qtxterm.keybindings_dialog import KeybindingsDialog from qtxterm.menu_prefs import ContextMenuOrderStore from qtxterm.preferences_dialog import PreferencesDialog from qtxterm.preset_menu import ( @@ -46,10 +49,16 @@ def __init__(self, settings: QSettings | None = None) -> None: self._apply_qt_theme() self._shell_store = ShellPreferenceStore(self._settings) self._menu_order_store = ContextMenuOrderStore(self._settings) + self._exit_store = PaneExitStore(self._settings) + # JSON beside presets.json rather than in the ini: a binding is a list + # of chords, and it is worth being hand-editable. + self._keybinding_store = KeybindingStore() self._tabs = TerminalTabWidget( parent=self, appearance_store=self._appearance_store, shell_store=self._shell_store, + exit_store=self._exit_store, + keybinding_store=self._keybinding_store, ) # Deliberately not wired to close(): the window outlives its # terminals. Closing the last one leaves an empty window you can open @@ -204,6 +213,9 @@ def _build_file_menu(self) -> None: browser_action.triggered.connect(lambda: self._tabs.new_browser_tab()) self._file_menu.addSeparator() + shortcuts_action = self._file_menu.addAction("Keyboard Shortcuts...") + shortcuts_action.triggered.connect(self.show_keybindings) + preferences_action = self._file_menu.addAction("Preferences...") preferences_action.triggered.connect(self.show_preferences) @@ -219,12 +231,16 @@ def _on_cron_job_failed(self, name: str, reason: str) -> None: """ self.statusBar().showMessage(f"Cron job {name!r}: {reason}", 10000) + def show_keybindings(self) -> None: + KeybindingsDialog(self._keybinding_store, self).exec() + def show_preferences(self) -> None: PreferencesDialog( self._appearance_store, self, shell_store=self._shell_store, order_store=self._menu_order_store, + exit_store=self._exit_store, ).exec() def closeEvent(self, event) -> None: diff --git a/src/qtxterm/pane.py b/src/qtxterm/pane.py index eaabb70..03be9b5 100644 --- a/src/qtxterm/pane.py +++ b/src/qtxterm/pane.py @@ -42,6 +42,16 @@ def shutdown(self) -> None: def apply_appearance(self, appearance) -> None: """Terminal theme and font. A no-op for panes they don't apply to.""" + def focus_pane(self) -> None: + """Put the keyboard in this pane. + + Overridden by panes that host a QWebEngineView, where focusing the + PaneWidget itself is not enough: the thing that actually receives + keystrokes is a Chromium child widget, and inside a terminal it is an + element inside the page below that. + """ + self.setFocus() + def set_pane_state(self, in_split: bool, active: bool) -> None: """Tell the pane whether it shares its tab, and whether it is focused. diff --git a/src/qtxterm/preferences_dialog.py b/src/qtxterm/preferences_dialog.py index 6a8d9e2..ad3990a 100644 --- a/src/qtxterm/preferences_dialog.py +++ b/src/qtxterm/preferences_dialog.py @@ -1,28 +1,41 @@ from __future__ import annotations +from pathlib import Path + from PySide6.QtCore import Qt from PySide6.QtGui import QFont from PySide6.QtWidgets import ( QComboBox, QDialog, QDialogButtonBox, + QFileDialog, QFontComboBox, QFormLayout, QHBoxLayout, + QLabel, + QLineEdit, QListWidget, QListWidgetItem, QPushButton, + QSlider, QSpinBox, QVBoxLayout, QWidget, ) from qtxterm.appearance import ( + MAX_BACKGROUND_OPACITY, + MAX_FONT_SIZE, MAX_SCROLLBACK, + MIN_BACKGROUND_OPACITY, + MIN_FONT_SIZE, MIN_SCROLLBACK, Appearance, AppearanceStore, ) +from qtxterm.exit_prefs import CHOICES as EXIT_CHOICES +from qtxterm.exit_prefs import LABELS as EXIT_LABELS +from qtxterm.exit_prefs import PaneExitStore from qtxterm.menu_prefs import SECTION_LABELS, ContextMenuOrderStore from qtxterm.shell_prefs import ( SYSTEM_DEFAULT, @@ -52,6 +65,7 @@ def __init__( parent: QWidget | None = None, shell_store: ShellPreferenceStore | None = None, order_store: ContextMenuOrderStore | None = None, + exit_store: PaneExitStore | None = None, ) -> None: super().__init__(parent) self.setWindowTitle("Preferences") @@ -61,6 +75,7 @@ def __init__( self._store = store self._shell_store = shell_store self._order_store = order_store + self._exit_store = exit_store layout = QVBoxLayout(self) form = QFormLayout() @@ -87,10 +102,55 @@ def __init__( form.addRow("Font", self._font_combo) self._size_spin = QSpinBox() - self._size_spin.setRange(6, 72) + self._size_spin.setRange(MIN_FONT_SIZE, MAX_FONT_SIZE) self._size_spin.setValue(store.current.font_size) form.addRow("Font size", self._size_spin) + if exit_store is not None: + self._exit_combo = QComboBox() + for choice in EXIT_CHOICES: + self._exit_combo.addItem(EXIT_LABELS[choice], choice) + index = self._exit_combo.findData(exit_store.current) + self._exit_combo.setCurrentIndex(max(index, 0)) + # Phrased as the question it answers, because "Close on exit" + # alone reads as a checkbox and gives no clue that the middle + # option - the useful one - exists at all. + form.addRow("When a shell exits", self._exit_combo) + + # Background image, as a row of [path][Browse...][Clear]. A plain + # line edit as well as the picker, because a path is worth being able + # to paste or hand-edit. + self._background_edit = QLineEdit(store.current.background_image) + self._background_edit.setPlaceholderText("None") + browse = QPushButton("Browse...") + browse.clicked.connect(self._pick_background_image) + clear = QPushButton("Clear") + clear.clicked.connect(self._background_edit.clear) + background_row = QHBoxLayout() + background_row.addWidget(self._background_edit, 1) + background_row.addWidget(browse) + background_row.addWidget(clear) + form.addRow("Background image", background_row) + + self._background_opacity_slider = QSlider(Qt.Orientation.Horizontal) + self._background_opacity_slider.setRange( + MIN_BACKGROUND_OPACITY, MAX_BACKGROUND_OPACITY + ) + self._background_opacity_slider.setValue(store.current.background_opacity) + self._background_opacity_label = QLabel() + # A slider with no readout leaves you guessing what you set, and the + # value is the whole point of this control. + self._background_opacity_slider.valueChanged.connect( + lambda value: self._background_opacity_label.setText(f"{value}%") + ) + self._background_opacity_label.setText( + f"{self._background_opacity_slider.value()}%" + ) + opacity_row = QHBoxLayout() + opacity_row.addWidget(self._background_opacity_slider, 1) + opacity_row.addWidget(self._background_opacity_label) + form.addRow("Image strength", opacity_row) + self._scrollback_spin = QSpinBox() self._scrollback_spin.setRange(MIN_SCROLLBACK, MAX_SCROLLBACK) self._scrollback_spin.setSingleStep(500) @@ -169,17 +229,34 @@ def _section_order(self) -> list[str]: for row in range(self._order_list.count()) ] + def _pick_background_image(self) -> None: + """Choose an image file, starting where the current one lives.""" + current = self._background_edit.text().strip() + start = str(Path(current).parent) if current else "" + path, _filter = QFileDialog.getOpenFileName( + self, + "Background image", + start, + "Images (*.png *.jpg *.jpeg *.bmp *.gif *.webp);;All files (*)", + ) + if path: + self._background_edit.setText(path) + def _save(self) -> None: if self._order_store is not None: self._order_store.save(self._section_order()) if self._shell_store is not None: self._shell_store.save(self._shell_combo.currentData()) + if self._exit_store is not None: + self._exit_store.save(self._exit_combo.currentData()) self._store.save( Appearance( theme_name=self._theme_combo.currentText(), font_family=self._font_combo.currentFont().family(), font_size=self._size_spin.value(), scrollback=self._scrollback_spin.value(), + background_image=self._background_edit.text().strip(), + background_opacity=self._background_opacity_slider.value(), ) ) self.accept() diff --git a/src/qtxterm/shortcuts.py b/src/qtxterm/shortcuts.py new file mode 100644 index 0000000..7b34c4b --- /dev/null +++ b/src/qtxterm/shortcuts.py @@ -0,0 +1,233 @@ +"""Keyboard shortcuts, resolved per platform. + +Qt's key sequence strings are already portable in one particular way: `Ctrl` +means the **Command** key on macOS and the Control key everywhere else, `Meta` +means Control on macOS, and `Alt` is Option there. That mapping does half the +job for free and quietly ruins the other half, which is why the table below +spells out both sides rather than leaning on it. + +Two facts drive every choice here: + +- **On Windows and Linux a terminal cannot use plain `Ctrl`+letter.** The + shell owns that space: `Ctrl+C` interrupts, `Ctrl+W` deletes a word, + `Ctrl+F` moves forward a character. So the app's own actions take + `Ctrl+Shift`, which is what Windows Terminal, GNOME Terminal and VS Code's + terminal all do. +- **On macOS the opposite is true.** The shell uses Control, and Command is + free, so the native binding is `Cmd`+letter with no Shift - `Cmd+T`, + `Cmd+W`, `Cmd+F`, exactly like every other Mac app. Writing `Ctrl+Shift+T` + and letting Qt translate it would produce `Cmd+Shift+T`, which on a Mac is + a different gesture entirely (in browsers it reopens a closed tab). + +The traps that make this more than a table: + +- `Ctrl+Tab` on macOS translates to **Cmd+Tab**, the OS application switcher, + so "next tab" has to be spelled `Meta+Tab` there to mean a physical + Ctrl+Tab. +- `Ctrl+C` on macOS translates to Cmd+C, which is genuinely copy. The + interrupt is physical Control+C, spelled `Meta+C`, and is deliberately left + unbound so it reaches the shell. +- Punctuation chords arrive as different Qt keys depending on the modifier + held, so several actions list the same chord twice under both spellings. + See SPLIT_RIGHT/SPLIT_DOWN and the comment on ZOOM_OUT. + +Some actions list more than one sequence, and the reason differs between +them. The distinction is worth keeping straight, because it decides whether +a second entry is dead weight: + +- A **compatibility hedge** is two spellings of *one* physical gesture, where + only ever one of them can fire - "Alt+Shift+=" and "Alt+Shift++" are the + same keypress, and which one Qt reports depends on the layout. Removing + either risks the chord doing nothing at all on some machine. +- An **alternative** is two genuinely different gestures deliberately mapped + to one action - ZOOM_IN takes both Ctrl+= and Ctrl++ (that is, + Ctrl+Shift+=), because people reach for the plus key with Shift held + without thinking, and every browser accepts both. Both really do work, and + that is the intent rather than an oversight. +""" + +from __future__ import annotations + +import sys + +# Read once. Tests override it directly rather than patching sys.platform, +# which PySide6 has already read by the time a test runs. +IS_MAC = sys.platform == "darwin" + +NEW_TAB = "new_tab" +CLOSE_TAB = "close_tab" +NEXT_TAB = "next_tab" +PREV_TAB = "prev_tab" +FIND = "find" +COPY = "copy" +PASTE = "paste" +ZOOM_IN = "zoom_in" +ZOOM_OUT = "zoom_out" +ZOOM_RESET = "zoom_reset" +SPLIT_RIGHT = "split_right" +SPLIT_DOWN = "split_down" +CLOSE_PANE = "close_pane" +FOCUS_PANE_LEFT = "focus_pane_left" +FOCUS_PANE_RIGHT = "focus_pane_right" +FOCUS_PANE_UP = "focus_pane_up" +FOCUS_PANE_DOWN = "focus_pane_down" + +# action -> (Windows/Linux, macOS). Both are lists because one action often +# needs several spellings to be reachable at all. +_TABLE: dict[str, tuple[list[str], list[str]]] = { + NEW_TAB: (["Ctrl+Shift+T"], ["Ctrl+T"]), + CLOSE_TAB: (["Ctrl+Shift+W"], ["Ctrl+W"]), + # Ctrl+Tab is the cross-platform gesture, but on macOS it must be spelled + # Meta+Tab or Qt hands it to Cmd+Tab, the OS app switcher. Cmd+Shift+[ ] + # is the Mac-native pair and is offered alongside. + NEXT_TAB: (["Ctrl+Tab"], ["Meta+Tab", "Ctrl+Shift+]"]), + PREV_TAB: (["Ctrl+Shift+Tab"], ["Meta+Shift+Tab", "Ctrl+Shift+["]), + FIND: (["Ctrl+Shift+F"], ["Ctrl+F"]), + # Ctrl+Insert / Shift+Insert are the older Windows and X11 gestures, still + # muscle memory for a lot of people and free of any shell meaning. + COPY: (["Ctrl+Shift+C", "Ctrl+Ins"], ["Ctrl+C"]), + PASTE: (["Ctrl+Shift+V", "Shift+Ins"], ["Ctrl+V"]), + # An alternative, not a hedge: Ctrl+= and Ctrl++ are different gestures + # (the second holds Shift) and both are meant to work, matching Chrome, + # Firefox, VS Code and Windows Terminal. Zoom out needs no counterpart - + # nobody reaches for Shift to press minus. + ZOOM_IN: (["Ctrl+=", "Ctrl++"], ["Ctrl+=", "Ctrl++"]), + # Deliberately *not* Ctrl+Shift+- as well: that is the same chord as + # SPLIT_DOWN, and binding one sequence to two actions makes whichever + # shortcut was registered second silently dead. + ZOOM_OUT: (["Ctrl+-"], ["Ctrl+-"]), + ZOOM_RESET: (["Ctrl+0"], ["Ctrl+0"]), + # Four spellings on Windows/Linux, for two different reasons. Alt+Shift+= + # is matched against Key_Equal by Qt while the keyboard sends Key_Plus; + # and with Ctrl held the character is a control code, so Qt cannot derive + # "|" from the layout and reports the base key, Key_Backslash. macOS gets + # iTerm2's Cmd+D / Cmd+Shift+D, which every Mac terminal user knows. + SPLIT_RIGHT: ( + ["Alt+Shift+=", "Alt+Shift++", "Ctrl+Shift+|", "Ctrl+Shift+\\"], + ["Ctrl+D"], + ), + SPLIT_DOWN: ( + ["Alt+Shift+-", "Alt+Shift+_", "Ctrl+Shift+_", "Ctrl+Shift+-"], + ["Ctrl+Shift+D"], + ), + CLOSE_PANE: (["Alt+Shift+W"], ["Ctrl+Shift+W"]), + # Alt+Arrow on Windows/Linux; macOS needs Cmd+Opt+Arrow, because plain + # Option+Arrow is word-wise cursor movement in every Mac shell. + FOCUS_PANE_LEFT: (["Alt+Left"], ["Ctrl+Alt+Left"]), + FOCUS_PANE_RIGHT: (["Alt+Right"], ["Ctrl+Alt+Right"]), + FOCUS_PANE_UP: (["Alt+Up"], ["Ctrl+Alt+Up"]), + FOCUS_PANE_DOWN: (["Alt+Down"], ["Ctrl+Alt+Down"]), +} + +# How many tabs are reachable by number. The ninth means "the last tab", +# following browsers and Windows Terminal, so it stays useful past nine tabs. +TAB_SLOTS = 9 +LAST_TAB_SLOT = TAB_SLOTS + + +def tab_slot_action(slot: int) -> str: + return f"tab_{slot}" + + +for _slot in range(1, TAB_SLOTS + 1): + _TABLE[tab_slot_action(_slot)] = ( + # Alternatives, deliberately: Alt+N is GNOME Terminal's and + # Ctrl+Alt+N is Windows Terminal's, and people arrive here with one + # or the other already in their fingers. Neither collides with a + # shell binding, so supporting both costs nothing. + [f"Alt+{_slot}", f"Ctrl+Alt+{_slot}"], + [f"Ctrl+{_slot}"], + ) + + +# Qt's names for the modifiers are not the names a Mac user sees. Qt "Ctrl" +# is the Command key there, "Meta" is Control and "Alt" is Option - so the +# sequence Qt calls "Ctrl+T" appears on screen, in menus and in the docs as +# "Cmd+T". Anything comparing a binding against user-facing text has to +# translate first, which is what caught the usage guide out on macOS CI: the +# guide correctly said Cmd+T and the test correctly asked for Ctrl+T. +# +# Substituting the "Mod+" form rather than the bare word keeps "Ctrl++" +# (zoom in, Ctrl and the plus key) intact, where matching on the word alone +# would have to reason about which trailing + is a separator. +# +# Order matters: Ctrl becomes Cmd first, so the Meta rule that follows +# cannot rewrite a Ctrl that was already translated. +_MAC_DISPLAY_NAMES = (("Ctrl+", "Cmd+"), ("Meta+", "Ctrl+"), ("Alt+", "Opt+")) + + +def display_sequence(sequence: str) -> str: + """A key sequence written the way this platform's users see it. + + The identity everywhere except macOS, where Qt's modifier names and the + keys people actually press are shuffled three ways. + """ + if not IS_MAC: + return sequence + for qt_name, shown in _MAC_DISPLAY_NAMES: + sequence = sequence.replace(qt_name, shown) + return sequence + + +def display_sequences_for(action: str) -> list[str]: + """Every sequence for `action`, named as the user sees it.""" + return [display_sequence(sequence) for sequence in sequences_for(action)] + + +# What each action is called in the shortcuts editor. Kept beside the table so +# a new action cannot be added without a name to show for it - an editor row +# reading "focus_pane_left" would be nobody's idea of a preference. +ACTION_LABELS = { + NEW_TAB: "New tab", + CLOSE_TAB: "Close tab", + NEXT_TAB: "Next tab", + PREV_TAB: "Previous tab", + FIND: "Find in the scrollback", + COPY: "Copy", + PASTE: "Paste", + ZOOM_IN: "Bigger text", + ZOOM_OUT: "Smaller text", + ZOOM_RESET: "Default text size", + SPLIT_RIGHT: "Split right", + SPLIT_DOWN: "Split down", + CLOSE_PANE: "Close pane", + FOCUS_PANE_LEFT: "Focus pane left", + FOCUS_PANE_RIGHT: "Focus pane right", + FOCUS_PANE_UP: "Focus pane up", + FOCUS_PANE_DOWN: "Focus pane down", +} + +for _slot in range(1, TAB_SLOTS + 1): + ACTION_LABELS[tab_slot_action(_slot)] = ( + "Go to the last tab" if _slot == LAST_TAB_SLOT else f"Go to tab {_slot}" + ) + + +def label_for(action: str) -> str: + return ACTION_LABELS.get(action, action) + + +def sequences_for(action: str) -> list[str]: + """Every key sequence that should trigger `action` on this platform.""" + other, mac = _TABLE[action] + return list(mac if IS_MAC else other) + + +def all_actions() -> list[str]: + return list(_TABLE) + + +def conflicts() -> dict[str, list[str]]: + """Sequences bound to more than one action on this platform. + + Exists because the failure it catches is invisible: two QShortcuts sharing + a sequence on the same widget makes Qt fire neither (it reports the + ambiguity and gives up), so the shortcut simply stops working with nothing + logged and no exception. A test calls this rather than trusting the table + to have been read carefully. + """ + seen: dict[str, list[str]] = {} + for action in _TABLE: + for sequence in sequences_for(action): + seen.setdefault(sequence, []).append(action) + return {seq: actions for seq, actions in seen.items() if len(actions) > 1} diff --git a/src/qtxterm/terminal_bridge.py b/src/qtxterm/terminal_bridge.py index b5d50d0..c65ef29 100644 --- a/src/qtxterm/terminal_bridge.py +++ b/src/qtxterm/terminal_bridge.py @@ -20,6 +20,7 @@ class needing to know about PTYs at all. script_loaded = Signal() title_changed = Signal(str) selection_changed = Signal(str) + link_activated = Signal(str) @Slot(str) def sendInput(self, data: str) -> None: @@ -60,3 +61,14 @@ def setSelection(self, text: str) -> None: is about to be shown. """ self.selection_changed.emit(text) + + @Slot(str) + def openLink(self, uri: str) -> None: + """A URL in the output was Ctrl+clicked. + + The URI is whatever the terminal printed, which on an SSH session is + whatever the remote host printed - so it is untrusted, and + TerminalWidget checks the scheme before handing it to the OS rather + than trusting the link addon's regex to be the only gate. + """ + self.link_activated.emit(uri) diff --git a/src/qtxterm/terminal_tabs.py b/src/qtxterm/terminal_tabs.py index 4bf5417..9556569 100644 --- a/src/qtxterm/terminal_tabs.py +++ b/src/qtxterm/terminal_tabs.py @@ -1,8 +1,9 @@ from __future__ import annotations import html +from dataclasses import replace -from PySide6.QtCore import QPoint, Qt, QTimer, Signal +from PySide6.QtCore import QPoint, QRect, Qt, QTimer, Signal from PySide6.QtGui import QKeySequence, QPainter, QPalette, QShortcut from PySide6.QtWidgets import ( QApplication, @@ -13,8 +14,17 @@ QWidget, ) -from qtxterm.appearance import Appearance, AppearanceStore +from qtxterm import shortcuts +from qtxterm.appearance import ( + DEFAULT_FONT_SIZE, + MAX_FONT_SIZE, + MIN_FONT_SIZE, + Appearance, + AppearanceStore, +) from qtxterm.browser_widget import BrowserWidget +from qtxterm.exit_prefs import PaneExitStore, should_close +from qtxterm.keybindings import KeybindingStore from qtxterm.pane import PaneWidget from qtxterm.presets import STEP_RIGHT, STEP_TAB, macro_steps from qtxterm.pty_backend import PtySession, default_shell @@ -40,9 +50,14 @@ def __init__( parent: QWidget | None = None, appearance_store: AppearanceStore | None = None, shell_store: ShellPreferenceStore | None = None, + exit_store: PaneExitStore | None = None, + keybinding_store: KeybindingStore | None = None, ) -> None: super().__init__(parent) self._shell_store = shell_store + self._exit_store = exit_store + self._keybinding_store = keybinding_store + self._shortcuts: list[QShortcut] = [] # Two layers: the automatic name (shell name, or a browser tab's # host) and an optional user-set one that overrides it. Kept apart so # a rename isn't silently overwritten the next time the automatic name @@ -71,6 +86,11 @@ def __init__( self.setCornerWidget(add_button, Qt.Corner.TopRightCorner) self._install_shortcuts() + if self._keybinding_store is not None: + # Rebuilt rather than patched: working out which QShortcuts a + # changed table implies is more code than simply making them all + # again, and the set is small. + self._keybinding_store.changed.connect(self._install_shortcuts) # focusChanged rather than an event filter per terminal: keyboard # focus inside a terminal lands on a Chromium child widget, not on the @@ -100,25 +120,70 @@ def paintEvent(self, event) -> None: ) def _install_shortcuts(self) -> None: - def bind(sequence: str, slot) -> QShortcut: - shortcut = QShortcut(QKeySequence(sequence), self) - shortcut.setContext(Qt.ShortcutContext.ApplicationShortcut) - shortcut.activated.connect(slot) - return shortcut - - self._new_tab_shortcut = bind("Ctrl+Shift+T", lambda: self.new_tab()) - self._close_tab_shortcut = bind("Ctrl+Shift+W", self._close_current_tab) - self._next_tab_shortcut = bind("Ctrl+Tab", self._activate_next_tab) - self._prev_tab_shortcut = bind("Ctrl+Shift+Tab", self._activate_prev_tab) - # Alt+Shift chords, following Windows Terminal: shells and TUIs rarely - # bind them, unlike Ctrl+Shift which readline and editors do use. - self._split_right_shortcut = bind( - "Alt+Shift+=", lambda: self.split_active(Qt.Orientation.Horizontal) + """Bind every action, using the sequences chosen for this platform. + + Application context on purpose: a QShortcut is matched before the + focused widget sees the key, which is what keeps the QTabBar from + treating a bare arrow as "switch tab" while a pane has focus. + """ + + # Dispose of the previous set first, or every rebind would leave its + # predecessor behind - still matched, still firing, and eventually + # ambiguous with whatever replaced it. + for stale in self._shortcuts: + stale.setEnabled(False) + stale.setParent(None) + stale.deleteLater() + self._shortcuts = [] + + def bind(sequences: list[str], slot) -> list[QShortcut]: + bound = [] + for sequence in sequences: + shortcut = QShortcut(QKeySequence(sequence), self) + shortcut.setContext(Qt.ShortcutContext.ApplicationShortcut) + shortcut.activated.connect(slot) + bound.append(shortcut) + return bound + + def seq(action: str) -> list[str]: + # Through the store when there is one, so a user override wins; + # straight from the table otherwise, which is what tests and + # embedders get. + if self._keybinding_store is not None: + return self._keybinding_store.sequences_for(action) + return shortcuts.sequences_for(action) + + def register(action: str, slot) -> None: + self._shortcuts.extend(bind(seq(action), slot)) + + register(shortcuts.NEW_TAB, lambda: self.new_tab()) + register(shortcuts.CLOSE_TAB, self._close_current_tab) + register(shortcuts.NEXT_TAB, self._activate_next_tab) + register(shortcuts.PREV_TAB, self._activate_prev_tab) + register( + shortcuts.SPLIT_RIGHT, + lambda: self.split_active(Qt.Orientation.Horizontal), ) - self._split_down_shortcut = bind( - "Alt+Shift+-", lambda: self.split_active(Qt.Orientation.Vertical) + register( + shortcuts.SPLIT_DOWN, + lambda: self.split_active(Qt.Orientation.Vertical), ) - self._close_pane_shortcut = bind("Alt+Shift+W", self.close_active_pane) + register(shortcuts.CLOSE_PANE, self.close_active_pane) + register(shortcuts.FIND, self.show_find_in_active) + register(shortcuts.COPY, self.copy_in_active) + register(shortcuts.PASTE, self.paste_in_active) + register(shortcuts.ZOOM_IN, lambda: self.zoom_font(1)) + register(shortcuts.ZOOM_OUT, lambda: self.zoom_font(-1)) + register(shortcuts.ZOOM_RESET, self.reset_font_zoom) + register(shortcuts.FOCUS_PANE_LEFT, lambda: self.focus_pane_in_direction(-1, 0)) + register(shortcuts.FOCUS_PANE_RIGHT, lambda: self.focus_pane_in_direction(1, 0)) + register(shortcuts.FOCUS_PANE_UP, lambda: self.focus_pane_in_direction(0, -1)) + register(shortcuts.FOCUS_PANE_DOWN, lambda: self.focus_pane_in_direction(0, 1)) + for slot in range(1, shortcuts.TAB_SLOTS + 1): + register( + shortcuts.tab_slot_action(slot), + lambda checked=False, s=slot: self.activate_tab_slot(s), + ) def _make_terminal( self, @@ -147,6 +212,9 @@ def _make_terminal( lambda title, w=widget: self._update_tab_tooltip(w, title) ) widget.context_menu_requested.connect(self.context_menu_requested) + widget.process_exited.connect( + lambda code, w=widget: self._on_process_exited(w, code) + ) return widget def new_tab( @@ -174,6 +242,11 @@ def _add_tab(self, widget): index = self.addTab(widget, "") self.setCurrentIndex(index) self._renumber() + # addTab leaves Qt's focus on the tab bar, so a brand new terminal + # needs a click before it accepts a keystroke - and until that click, + # arrow keys switch tabs instead. + widget.focus_pane() + self._schedule_background_refresh() return widget @staticmethod @@ -216,6 +289,9 @@ def _apply_appearance_to_all_tabs(self) -> None: for i in range(self.count()): for pane in self._panes_in(self.widget(i)): pane.apply_appearance(appearance) + # A new image needs its slices recomputed; a new theme needs the veil + # repainted over them. + self._schedule_background_refresh() def close_tab_at(self, index: int) -> None: widget = self.widget(index) @@ -368,12 +444,22 @@ def split_active( splitter.widget(i).show() self._even_out(splitter) + # A new pane changes every sibling's slice of the background. + splitter.splitterMoved.connect( + lambda *_args: self.refresh_background_geometry() + ) # Again once the layout has run: at this point the splitter has just # been inserted and has no geometry, so its extent is 0 and there is # nothing to divide yet. QTimer.singleShot(0, lambda s=splitter: self._even_out(s)) self._focused_panes[self.widget(index)] = new_terminal self._refresh_pane_indicators() + self._schedule_background_refresh() + # Qt leaves focus on the tab bar after the tab surgery above, which + # is worse than it sounds: the new pane looks active and swallows + # nothing, while arrow keys reach the QTabBar and switch *tabs*. The + # pane you just made is the one you want to type in, so focus it. + new_terminal.focus_pane() return new_terminal @staticmethod @@ -461,7 +547,15 @@ def move_active_pane_to_new_tab(self) -> TerminalWidget | None: def close_active_pane(self) -> None: """Close the focused pane; the last pane closes the whole tab.""" - terminal = self.active_pane() + self.close_pane(self.active_pane()) + + def close_pane(self, terminal: PaneWidget | None) -> None: + """Close one pane; the last pane in a tab closes the tab. + + Takes the pane rather than assuming the focused one, because a shell + that exits does so in whichever pane it was running - very often not + the one you are looking at. + """ if terminal is None: return index = self.tab_index_of(terminal) @@ -481,6 +575,10 @@ def close_active_pane(self) -> None: self._focused_panes[self.widget(index)] = remaining[0] self._collapse_single_child_splitters(index) self._refresh_pane_indicators() + # Closing the focused pane leaves the keyboard nowhere. Hand it to + # the pane that inherited the space. + remaining[0].focus_pane() + self._schedule_background_refresh() def _collapse_single_child_splitters(self, index: int) -> None: """Unwrap splitters left holding one pane, so the tree doesn't grow @@ -625,6 +723,209 @@ def active_terminal(self) -> TerminalWidget | None: pane = self.active_pane() return pane if isinstance(pane, TerminalWidget) else None + def _pane_rect(self, pane: PaneWidget) -> QRect: + """A pane's geometry in this widget's coordinates. + + Mapped rather than read directly so panes nested at different depths + of the splitter tree are comparable with each other. + """ + return QRect(pane.mapTo(self, pane.rect().topLeft()), pane.size()) + + def focus_pane_in_direction(self, dx: int, dy: int) -> PaneWidget | None: + """Move the keyboard to the neighbouring pane in a direction. + + Geometry, not tree order: `dx`/`dy` are a unit direction and panes are + compared by where they sit on screen, so Alt+Right in a nested split + lands on the pane genuinely to the right rather than on whatever comes + next in the splitter tree. + + Candidates are ranked by how much of their edge lines up with the + current pane, then by distance, then top-left first. Ranking by + distance between centres looked equivalent and was not: with a tall + pane on the left and two stacked on the right, the two candidates' + centres sat 138 and 139 pixels off axis, so which one Alt+Right chose + came down to a single pixel and would flip if the splitter moved. + Overlap is stable under that, and the final top-left tie-break means + an exact tie still resolves the same way every time. + """ + current = self.active_pane() + tab = self.currentWidget() + panes = self._panes_in(tab) + if current is None or len(panes) < 2: + return None + + origin = self._pane_rect(current) + best: PaneWidget | None = None + best_score: tuple[int, int, int, int] | None = None + for pane in panes: + if pane is current: + continue + rect = self._pane_rect(pane) + if dx: + along = (rect.center().x() - origin.center().x()) * dx + overlap = min(rect.bottom(), origin.bottom()) - max( + rect.top(), origin.top() + ) + else: + along = (rect.center().y() - origin.center().y()) * dy + overlap = min(rect.right(), origin.right()) - max( + rect.left(), origin.left() + ) + # Strictly beyond us in the direction asked for, and sharing at + # least some edge - a pane diagonally opposite is not "to the + # right", and stepping to it would be a jump nobody predicted. + if along <= 0 or overlap <= 0: + continue + score = (-overlap, along, rect.top(), rect.left()) + if best_score is None or score < best_score: + best, best_score = pane, score + + if best is None: + return None + self._focused_panes[tab] = best + best.focus_pane() + self._refresh_pane_indicators() + return best + + def _on_process_exited(self, terminal: TerminalWidget, exit_code: int) -> None: + """Close the pane whose shell just exited, if the preference says so. + + Deferred to the next turn of the event loop rather than closed here: + this runs from the PTY's own `exited` signal, and deleting the widget + that owns the object currently emitting is how you get a crash rather + than a closed pane. The pane is also re-checked at that point, since + a tab closed in the meantime would leave nothing to act on. + """ + if self._exit_store is None: + return + if not should_close(self._exit_store.current, exit_code): + return + QTimer.singleShot(0, lambda: self._close_exited_pane(terminal)) + + def _close_exited_pane(self, terminal: TerminalWidget) -> None: + if self.tab_index_of(terminal) == -1: + return + self.close_pane(terminal) + + def refresh_background_geometry(self) -> None: + """Push every terminal pane its rectangle within its own tab. + + Cheap enough to do for all tabs rather than only the visible one: a + background tab that is resized while hidden would otherwise paint a + stale slice the moment you switched to it. + """ + for index in range(self.count()): + page = self.widget(index) + if page is None: + continue + for pane in self._panes_in(page): + if not isinstance(pane, TerminalWidget): + continue + # The view, not the pane: the pane's border margin is not part + # of the page, so using it would shift every slice by two + # pixels and leave visible seams between panes. + origin = pane.view_origin_in(page) + pane.set_background_geometry( + origin.x(), origin.y(), page.width(), page.height() + ) + + def _schedule_background_refresh(self) -> None: + """Refresh now and again after the layout settles. + + A pane inserted into a splitter has no geometry yet, so the immediate + pass would push zeros; the deferred one catches the real numbers. The + same trick, and the same reason, as _even_out. + """ + self.refresh_background_geometry() + QTimer.singleShot(0, self.refresh_background_geometry) + + def resizeEvent(self, event) -> None: + super().resizeEvent(event) + self._schedule_background_refresh() + + def copy_in_active(self) -> bool: + """Copy the focused terminal's selection. False if there was none. + + Returning rather than always swallowing the key matters for the + macOS binding: there Cmd+C is copy, and with nothing selected the + sensible thing is to do nothing at all rather than clear the + clipboard. + """ + terminal = self.active_terminal() + return bool(terminal and terminal.copy_selection()) + + def paste_in_active(self) -> None: + """Paste into the focused terminal, honouring bracketed paste. + + Routed through the terminal rather than written to the PTY directly + so a multi-line clipboard arrives as a paste, not as a series of + typed commands with their newlines acted on. + """ + terminal = self.active_terminal() + if terminal is not None: + terminal.paste_from_clipboard() + + def zoom_font(self, steps: int) -> int | None: + """Grow or shrink the terminal font, and remember it. + + Saved through the appearance store rather than pushed straight at the + open terminals, so it survives a restart and reaches panes opened + later - the same path the Preferences font size uses, which also + means the shell is told its new grid size rather than left wrapping + to the old width. + """ + if self._appearance_store is None: + return None + current = self._appearance_store.current + size = max(MIN_FONT_SIZE, min(current.font_size + steps, MAX_FONT_SIZE)) + if size == current.font_size: + return size + self._appearance_store.save(replace(current, font_size=size)) + return size + + def reset_font_zoom(self) -> int | None: + """Back to the default font size, not to whatever Preferences last held. + + There is only one stored size, so zooming *is* editing the + preference; without a fixed point to return to, Ctrl+0 would have + nothing to mean. + """ + if self._appearance_store is None: + return None + current = self._appearance_store.current + if current.font_size != DEFAULT_FONT_SIZE: + self._appearance_store.save(replace(current, font_size=DEFAULT_FONT_SIZE)) + return DEFAULT_FONT_SIZE + + def activate_tab_slot(self, slot: int) -> bool: + """Switch to the tab in position `slot`, counting from 1. + + The last slot means the *last* tab rather than the ninth, following + browsers and Windows Terminal, so it keeps working once there are + more tabs than slots. + """ + if self.count() == 0: + return False + index = self.count() - 1 if slot >= shortcuts.LAST_TAB_SLOT else slot - 1 + if not 0 <= index < self.count(): + return False + self.setCurrentIndex(index) + pane = self.active_pane() + if pane is not None: + pane.focus_pane() + return True + + def show_find_in_active(self) -> None: + """Open the find bar in the focused pane, if that pane is a terminal. + + No-op on a browser pane, for the same reason commands are: there is + no scrollback to search, and silently searching some other terminal in + the tab would put the bar somewhere you weren't looking. + """ + terminal = self.active_terminal() + if terminal is not None: + terminal.show_find() + def _set_tab_title(self, widget, title: str) -> None: """Update the automatic name. A user rename still wins over it.""" self._auto_titles[widget] = title diff --git a/src/qtxterm/terminal_widget.py b/src/qtxterm/terminal_widget.py index cd076e5..c232689 100644 --- a/src/qtxterm/terminal_widget.py +++ b/src/qtxterm/terminal_widget.py @@ -4,7 +4,7 @@ from pathlib import Path from PySide6.QtCore import QPoint, Qt, QUrl, QUrlQuery, Signal -from PySide6.QtGui import QColor, QGuiApplication +from PySide6.QtGui import QColor, QDesktopServices, QGuiApplication from PySide6.QtWebChannel import QWebChannel from PySide6.QtWebEngineWidgets import QWebEngineView from PySide6.QtWidgets import QVBoxLayout, QWidget @@ -16,6 +16,14 @@ ASSETS_DIR = Path(__file__).parent / "assets" +# What a Ctrl+click in the terminal is allowed to hand to the OS. The link +# addon's own regex already matches only http/https, but that regex is not a +# security boundary: the text it ran against came out of the terminal, which +# on an SSH session means it came from the remote host. QDesktopServices. +# openUrl will happily launch a registered handler for any scheme, so the +# check is repeated here, where it decides whether anything is launched. +OPENABLE_URL_SCHEMES = frozenset({"http", "https"}) + def shell_short_name(shell: str) -> str: """Best-effort short label for a shell path, e.g. 'powershell.exe' -> 'powershell'.""" @@ -30,6 +38,10 @@ class TerminalWidget(PaneWidget): title_changed = Signal(str) pty_started = Signal() + # The shell finished, with its exit code. Separate from the bridge's + # `exited`, which only tells the page to print a line: this one is + # what decides whether the pane closes itself. + process_exited = Signal(int) # Global position, so a listener can pop a menu up without knowing where # this widget sits. context_menu_requested = Signal(QPoint) @@ -78,17 +90,35 @@ def __init__( layout.addWidget(self._view) self._script_loaded = False + self._focus_when_loaded = False + self._pending_background_geometry: tuple[int, int, int, int] | None = None self._bridge.script_loaded.connect(self._on_script_loaded) self._bridge.terminal_ready.connect(self._on_terminal_ready) self._bridge.input_received.connect(self._pty.write) self._bridge.resize_requested.connect(self._pty.resize) self._bridge.title_changed.connect(self.title_changed.emit) self._bridge.selection_changed.connect(self._on_selection_changed) + self._bridge.link_activated.connect(self.open_link) self._pty.output_ready.connect(self._bridge.output.emit) self._pty.exited.connect(self._bridge.exited.emit) + self._pty.exited.connect(self.process_exited.emit) self._view.load(self._terminal_url(appearance or Appearance())) + @staticmethod + def _background_image_url(appearance: Appearance) -> str: + """The background image as a URL the page can load, or "". + + A plain filesystem path will not do: the page is served from file:// + and resolves a bare path relative to the assets directory. Missing + files resolve to "" rather than a broken url(), so deleting the image + leaves a normal terminal instead of a half-painted one. + """ + path = (appearance.background_image or "").strip() + if not path or not Path(path).is_file(): + return "" + return QUrl.fromLocalFile(str(Path(path).resolve())).toString() + @staticmethod def _terminal_url(appearance: Appearance) -> QUrl: # Passed as query params (rather than a post-load bridge call) so @@ -100,6 +130,10 @@ def _terminal_url(appearance: Appearance) -> QUrl: query.addQueryItem("fontFamily", appearance.font_family) query.addQueryItem("fontSize", str(appearance.font_size)) query.addQueryItem("scrollback", str(appearance.scrollback)) + query.addQueryItem( + "backgroundImage", TerminalWidget._background_image_url(appearance) + ) + query.addQueryItem("backgroundOpacity", str(appearance.background_opacity)) url.setQuery(query) return url @@ -112,6 +146,8 @@ def apply_appearance(self, appearance: Appearance) -> None: "fontFamily": appearance.font_family, "fontSize": appearance.font_size, "scrollback": appearance.scrollback, + "backgroundImage": self._background_image_url(appearance), + "backgroundOpacity": appearance.background_opacity, } ) self._view.page().runJavaScript( @@ -141,6 +177,17 @@ def _on_script_loaded(self) -> None: """ self._script_loaded = True self._apply_size() + # Geometry pushed before the page was ready was dropped, and nothing + # would push it again until the next split or resize - which left the + # first pane of a tab painting the whole background while its + # neighbours painted their slices. + if self._pending_background_geometry is not None: + pending = self._pending_background_geometry + self._pending_background_geometry = None + self.set_background_geometry(*pending) + if self._focus_when_loaded: + self._focus_when_loaded = False + self.focus_pane() def resizeEvent(self, event) -> None: super().resizeEvent(event) @@ -195,6 +242,85 @@ def paste(self, text: str) -> None: def paste_from_clipboard(self) -> None: self.paste(QGuiApplication.clipboard().text()) + def open_link(self, uri: str) -> bool: + """Open a URL Ctrl+clicked in the output, in the system browser. + + The system browser rather than a qtxterm browser tab, even though the + app has them. Two reasons, and the second is the one that settles it: + it is what every other terminal does, and the URL is untrusted output, + so the sandboxed browser the user already keeps their extensions and + blocklists in is the better place for it than this app's embedded + engine. + + Returns whether it was opened, so a caller can tell a refused scheme + from a successful launch. + """ + url = QUrl(uri.strip()) + if not url.isValid() or url.scheme().lower() not in OPENABLE_URL_SCHEMES: + return False + QDesktopServices.openUrl(url) + return True + + def view_origin_in(self, ancestor: QWidget) -> QPoint: + """Where this pane's *page* starts, in `ancestor` coordinates.""" + return self._view.mapTo(ancestor, QPoint(0, 0)) + + def set_background_geometry( + self, x: int, y: int, tab_width: int, tab_height: int + ) -> None: + """Tell the page where this pane sits inside its tab. + + A background image spans the whole tab, but every pane is a separate + page, so none of them can work this out alone - left to itself each + would paint the entire picture and a split tab would show it once per + pane. Only the tab widget knows the layout, so it pushes the numbers + here. + + Measured from the web view rather than the pane, because the pane + carries a border margin the page never sees. + """ + if not self._script_loaded: + self._pending_background_geometry = (x, y, tab_width, tab_height) + return + self._view.page().runJavaScript( + "window.applyBackgroundGeometry && window.applyBackgroundGeometry(" + f"{x}, {y}, {tab_width}, {tab_height});" + ) + + def focus_pane(self) -> None: + """Put the keyboard in this terminal. + + Two steps, because there are two layers between the pane and the + keys. `setFocus()` on the view moves Qt's focus off whatever had it - + the tab bar, after a split - and `term.focus()` moves it again inside + the page, to the hidden textarea xterm actually reads from. Without + the second, the pane looks focused and types nowhere. + + A pane split off a moment ago has no page yet, and runJavaScript + against it is silently dropped, so the request is remembered and + replayed from _on_script_loaded. + """ + self._view.setFocus() + if self._script_loaded: + self._view.page().runJavaScript( + "window.focusTerminal && window.focusTerminal();" + ) + else: + self._focus_when_loaded = True + + def show_find(self) -> None: + """Open the find bar over this terminal and focus its input. + + The bar is part of the page, not a Qt widget stacked above the view: + a Qt bar would take rows off the grid every time it appeared, and + reflowing the shell's output is a high price for opening a search. + """ + self._view.page().runJavaScript("window.showFind && window.showFind();") + + def hide_find(self) -> None: + """Close the find bar, clear its highlights, and refocus the terminal.""" + self._view.page().runJavaScript("window.hideFind && window.hideFind();") + def send_command(self, text: str) -> None: """Write a line to the PTY and submit it, as if the user typed it + Enter. diff --git a/tests/test_appearance.py b/tests/test_appearance.py index aceb063..8466b30 100644 --- a/tests/test_appearance.py +++ b/tests/test_appearance.py @@ -2,6 +2,8 @@ from __future__ import annotations +from dataclasses import replace + from pathlib import Path from PySide6.QtCore import QSettings @@ -96,3 +98,39 @@ def test_a_hand_edited_scrollback_is_clamped(tmp_path: Path) -> None: settings.setValue("appearance/scrollback", 10**9) assert AppearanceStore(make_settings(tmp_path)).current.scrollback == MAX_SCROLLBACK + + +def test_background_image_and_opacity_round_trip(tmp_path) -> None: + from qtxterm.appearance import DEFAULT_BACKGROUND_OPACITY + + settings = QSettings(str(tmp_path / "a.ini"), QSettings.Format.IniFormat) + store = AppearanceStore(settings) + assert store.current.background_image == "" + assert store.current.background_opacity == DEFAULT_BACKGROUND_OPACITY + + store.save(replace(store.current, background_image="C:/wall.png", background_opacity=70)) + + reloaded = AppearanceStore(QSettings(str(tmp_path / "a.ini"), QSettings.Format.IniFormat)) + assert reloaded.current.background_image == "C:/wall.png" + assert reloaded.current.background_opacity == 70 + + +def test_background_opacity_is_clamped_on_load(tmp_path) -> None: + """The ini is hand-editable, and a nonsense percentage should not produce + an invalid CSS alpha.""" + from qtxterm.appearance import MAX_BACKGROUND_OPACITY + + settings = QSettings(str(tmp_path / "a.ini"), QSettings.Format.IniFormat) + settings.setValue("appearance/backgroundOpacity", 900) + assert AppearanceStore(settings).current.background_opacity == MAX_BACKGROUND_OPACITY + + settings.setValue("appearance/backgroundOpacity", -5) + assert AppearanceStore(settings).current.background_opacity == 0 + + +def test_the_default_background_strength_keeps_text_readable() -> None: + """A photograph at full strength behind text is unreadable, so trying the + feature for the first time should still leave a working terminal.""" + from qtxterm.appearance import DEFAULT_BACKGROUND_OPACITY + + assert 0 < DEFAULT_BACKGROUND_OPACITY <= 50 diff --git a/tests/test_exit_prefs.py b/tests/test_exit_prefs.py new file mode 100644 index 0000000..c89428d --- /dev/null +++ b/tests/test_exit_prefs.py @@ -0,0 +1,75 @@ +"""Closing a pane when its shell exits.""" + +from __future__ import annotations + +import pytest +from PySide6.QtCore import QSettings + +from qtxterm.exit_prefs import ( + CLOSE_ALWAYS, + CLOSE_CLEAN, + CLOSE_NEVER, + DEFAULT, + PaneExitStore, + should_close, +) + + +def make_store(tmp_path) -> PaneExitStore: + settings = QSettings(str(tmp_path / "s.ini"), QSettings.Format.IniFormat) + return PaneExitStore(settings) + + +def test_clean_closes_only_on_a_zero_exit_code() -> None: + """The whole reason this is three settings and not a checkbox: a shell + that died has usually printed why, and closing the pane throws that away + exactly when you needed to read it.""" + assert should_close(CLOSE_CLEAN, 0) is True + assert should_close(CLOSE_CLEAN, 1) is False + assert should_close(CLOSE_CLEAN, 130) is False + + +def test_always_closes_whatever_happened() -> None: + assert should_close(CLOSE_ALWAYS, 0) is True + assert should_close(CLOSE_ALWAYS, 1) is True + + +def test_never_keeps_the_pane() -> None: + """What qtxterm did before this existed.""" + assert should_close(CLOSE_NEVER, 0) is False + assert should_close(CLOSE_NEVER, 1) is False + + +def test_the_default_keeps_a_failed_shell_on_screen() -> None: + assert DEFAULT == CLOSE_CLEAN + + +def test_the_choice_round_trips(tmp_path) -> None: + store = make_store(tmp_path) + assert store.current == DEFAULT + + store.save(CLOSE_ALWAYS) + + assert make_store(tmp_path).current == CLOSE_ALWAYS + + +def test_saving_emits_changed(tmp_path, qtbot) -> None: + store = make_store(tmp_path) + + with qtbot.waitSignal(store.changed, timeout=1000): + store.save(CLOSE_NEVER) + + +def test_an_unknown_stored_value_falls_back_rather_than_raising(tmp_path) -> None: + """The ini is hand-editable, and a typo should not stop the app starting.""" + settings = QSettings(str(tmp_path / "s.ini"), QSettings.Format.IniFormat) + settings.setValue("session/closeOnExit", "sometimes") + + assert PaneExitStore(settings).current == DEFAULT + + +def test_saving_an_unknown_choice_is_refused(tmp_path) -> None: + store = make_store(tmp_path) + + with pytest.raises(ValueError): + store.save("sometimes") diff --git a/tests/test_find_in_page.py b/tests/test_find_in_page.py new file mode 100644 index 0000000..e01af7f --- /dev/null +++ b/tests/test_find_in_page.py @@ -0,0 +1,180 @@ +"""The find bar, exercised against the real xterm.js search addon. + +The Python side of find is three lines that call into JavaScript, so testing +it in isolation only asserts that a string was sent. What is actually worth +knowing - that a query finds the right number of matches in the scrollback, +that the counter reads correctly, that Escape puts focus back in the terminal +- lives in the page, so these tests drive the page. + +Slower than the rest of the suite (each one boots a real QWebEngineView and +xterm.js) which is why there are few of them, covering behaviour rather than +every button. +""" + +from __future__ import annotations + +import json + +import pytest + +from conftest import FakePtySession + +from qtxterm.terminal_widget import TerminalWidget + +# Enough lines to prove a match is found in the scrollback rather than only on +# the visible screen, with a repeated word to count and a distinct one to miss. +SAMPLE_OUTPUT = ( + "".join(f"line {i} nothing here\r\n" for i in range(60)) + + "first error here\r\n" + + "second ERROR here\r\n" + + "third error here\r\n" +) + + +@pytest.fixture +def terminal(qtbot): + """A visible, booted terminal with SAMPLE_OUTPUT already written to it. + + Shown rather than merely constructed: the page is only told its size from + resizeEvent and the loaded() handshake, and a zero-sized view never starts + the terminal at all. + """ + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + widget.resize(800, 500) + widget.show() + qtbot.waitUntil(lambda: widget.is_pty_started, timeout=15000) + widget._bridge.output.emit(SAMPLE_OUTPUT) + # xterm.js buffers writes and flushes them on its own schedule, so the + # text is not searchable the instant it is handed over - searching here + # without waiting reports "No results" against a terminal that is about to + # contain three matches. The rendered rows are the honest signal that the + # write has landed. + qtbot.waitUntil( + lambda: ( + "third error here" + in evaluate( + widget, qtbot, 'document.querySelector(".xterm-rows").textContent' + ) + ), + timeout=10000, + ) + yield widget + + +def evaluate(widget, qtbot, script: str): + """Run `script` in the page and return its value. + + runJavaScript is asynchronous - there is a whole IPC round trip to the + render process - so the result arrives in a callback and the event loop + has to keep turning until it does. + """ + result = [] + widget._view.page().runJavaScript(script, result.append) + qtbot.waitUntil(lambda: bool(result), timeout=10000) + return result[0] + + +def search_for(widget, qtbot, query: str) -> None: + """Type `query` into the find bar the way a user would. + + The value is set and an `input` event dispatched rather than calling the + search directly, so the test goes through the same path typing does - + including the incremental option, which only the input handler passes. + """ + evaluate( + widget, + qtbot, + f""" + (function () {{ + window.showFind(); + const input = document.getElementById("find-input"); + input.value = {json.dumps(query)}; + input.dispatchEvent(new Event("input")); + return true; + }})(); + """, + ) + + +def count_text(widget, qtbot) -> str: + return evaluate(widget, qtbot, 'document.getElementById("find-count").textContent') + + +def test_find_counts_every_match_in_the_scrollback(terminal, qtbot) -> None: + """Three matches, two of them scrolled off the screen's top is not the + point - the point is that search covers the buffer, not the viewport.""" + search_for(terminal, qtbot, "error here") + + qtbot.waitUntil(lambda: count_text(terminal, qtbot) != "", timeout=10000) + assert count_text(terminal, qtbot) == "1 of 3" + + +def test_search_is_case_insensitive_until_match_case_is_pressed( + terminal, qtbot +) -> None: + """'ERROR' on the middle line is one of the three by default, and one of + two once case matters. + + Only the match *count* is asserted after the toggle. Which match is + active can legitimately step forward by one, because the addon resumes + the search from the current selection rather than from the top. + """ + search_for(terminal, qtbot, "error") + qtbot.waitUntil(lambda: count_text(terminal, qtbot) != "", timeout=10000) + assert count_text(terminal, qtbot) == "1 of 3" + + evaluate(terminal, qtbot, 'document.getElementById("find-case").click()') + + qtbot.waitUntil(lambda: count_text(terminal, qtbot).endswith("of 2"), timeout=10000) + + +def test_a_query_that_matches_nothing_says_so(terminal, qtbot) -> None: + search_for(terminal, qtbot, "no such text anywhere") + + qtbot.waitUntil(lambda: count_text(terminal, qtbot) == "No results", timeout=10000) + # The input is outlined in the theme's red as well, which is the part you + # actually notice; the class is what drives it. + assert evaluate( + terminal, + qtbot, + 'document.getElementById("find-bar").classList.contains("no-results")', + ) + + +def test_a_half_typed_regex_reports_no_results_instead_of_throwing( + terminal, qtbot +) -> None: + """'[a' is what a regex looks like one keystroke before it is valid, and + the addon throws on it rather than simply not matching.""" + evaluate(terminal, qtbot, 'document.getElementById("find-regex").click()') + search_for(terminal, qtbot, "[a") + + qtbot.waitUntil(lambda: count_text(terminal, qtbot) == "No results", timeout=10000) + + +def test_closing_the_find_bar_hides_it_and_clears_the_count(terminal, qtbot) -> None: + search_for(terminal, qtbot, "error") + qtbot.waitUntil(lambda: count_text(terminal, qtbot) != "", timeout=10000) + + evaluate(terminal, qtbot, "window.hideFind()") + + assert evaluate(terminal, qtbot, "window.isFindOpen()") is False + # Focus back in the terminal, or the window looks usable and swallows + # every keystroke. + assert evaluate( + terminal, + qtbot, + 'document.activeElement.classList.contains("xterm-helper-textarea")', + ) + + +def test_the_find_bar_is_themed_from_the_terminal_theme(terminal, qtbot) -> None: + """A find bar with fixed colours is a white box on a black terminal.""" + background = evaluate( + terminal, + qtbot, + 'document.documentElement.style.getPropertyValue("--find-bg")', + ) + + assert background == "#1e1e1e" diff --git a/tests/test_keybindings.py b/tests/test_keybindings.py new file mode 100644 index 0000000..5548acd --- /dev/null +++ b/tests/test_keybindings.py @@ -0,0 +1,187 @@ +"""User overrides for the keyboard shortcuts.""" + +from __future__ import annotations + +import json + +import pytest + +from qtxterm import shortcuts +from qtxterm.keybindings import ConflictError, KeybindingStore, normalise + + +def make_store(tmp_path) -> KeybindingStore: + return KeybindingStore(path=tmp_path / "keybindings.json") + + +def test_an_untouched_action_follows_the_defaults(tmp_path) -> None: + store = make_store(tmp_path) + + assert store.sequences_for(shortcuts.NEW_TAB) == shortcuts.sequences_for( + shortcuts.NEW_TAB + ) + assert store.is_customised(shortcuts.NEW_TAB) is False + + +def test_an_override_replaces_the_default(tmp_path) -> None: + store = make_store(tmp_path) + + store.set_sequences(shortcuts.NEW_TAB, ["Ctrl+Alt+N"]) + + assert store.sequences_for(shortcuts.NEW_TAB) == ["Ctrl+Alt+N"] + assert store.is_customised(shortcuts.NEW_TAB) is True + + +def test_only_the_differences_are_written_to_disk(tmp_path) -> None: + """Saving the whole resolved table would freeze today's defaults into + every config file, so a later version that improves a binding would never + reach anyone who had opened the editor once.""" + store = make_store(tmp_path) + store.set_sequences(shortcuts.NEW_TAB, ["Ctrl+Alt+N"]) + + written = json.loads((tmp_path / "keybindings.json").read_text(encoding="utf-8")) + + assert list(written["bindings"]) == [shortcuts.NEW_TAB] + + +def test_an_override_survives_a_reload(tmp_path) -> None: + make_store(tmp_path).set_sequences(shortcuts.FIND, ["Ctrl+Alt+F"]) + + assert make_store(tmp_path).sequences_for(shortcuts.FIND) == ["Ctrl+Alt+F"] + + +def test_resetting_returns_to_the_default(tmp_path) -> None: + store = make_store(tmp_path) + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+F"]) + + store.reset(shortcuts.FIND) + + assert store.sequences_for(shortcuts.FIND) == shortcuts.sequences_for( + shortcuts.FIND + ) + assert store.is_customised(shortcuts.FIND) is False + + +def test_reset_all_clears_every_override(tmp_path) -> None: + store = make_store(tmp_path) + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+F"]) + store.set_sequences(shortcuts.NEW_TAB, ["Ctrl+Alt+N"]) + + store.reset_all() + + assert store.conflicts() == {} + assert not any(store.is_customised(a) for a in shortcuts.all_actions()) + + +def test_a_sequence_another_action_holds_is_refused(tmp_path) -> None: + """Two QShortcuts sharing a sequence makes Qt fire neither, so accepting + the newer binding last-wins would quietly disable both actions.""" + store = make_store(tmp_path) + taken = shortcuts.sequences_for(shortcuts.CLOSE_TAB)[0] + + with pytest.raises(ConflictError) as excinfo: + store.set_sequences(shortcuts.NEW_TAB, [taken]) + + assert excinfo.value.sequence == normalise(taken) + assert excinfo.value.action == shortcuts.CLOSE_TAB + # And the rejected binding did not take effect. + assert store.is_customised(shortcuts.NEW_TAB) is False + + +def test_an_action_may_keep_its_own_sequence_when_rebinding(tmp_path) -> None: + """Adding a second chord must not trip the conflict check against the + chord the action already had.""" + store = make_store(tmp_path) + existing = shortcuts.sequences_for(shortcuts.FIND)[0] + + store.set_sequences(shortcuts.FIND, [existing, "Ctrl+Alt+F"]) + + assert store.sequences_for(shortcuts.FIND) == [normalise(existing), "Ctrl+Alt+F"] + + +def test_duplicates_within_one_action_are_collapsed(tmp_path) -> None: + store = make_store(tmp_path) + + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+F", "ctrl+alt+f"]) + + assert store.sequences_for(shortcuts.FIND) == ["Ctrl+Alt+F"] + + +def test_sequences_are_normalised_so_case_does_not_matter(tmp_path) -> None: + """A hand-edited file should not be able to produce a binding that looks + set in the editor and never matches a key.""" + store = make_store(tmp_path) + + store.set_sequences(shortcuts.FIND, ["ctrl+shift+alt+f"]) + + assert store.sequences_for(shortcuts.FIND) == [normalise("Ctrl+Shift+Alt+F")] + + +def test_an_action_can_have_no_shortcut_at_all(tmp_path) -> None: + """Different from resetting: an action nobody wants a key for is a + legitimate thing to ask for.""" + store = make_store(tmp_path) + + store.set_sequences(shortcuts.FIND, []) + + assert store.sequences_for(shortcuts.FIND) == [] + assert store.is_customised(shortcuts.FIND) is True + + +def test_rubbish_is_refused(tmp_path) -> None: + store = make_store(tmp_path) + + with pytest.raises(ValueError): + store.set_sequences(shortcuts.FIND, ["not a chord at all"]) + + +def test_an_unknown_action_is_refused(tmp_path) -> None: + store = make_store(tmp_path) + + with pytest.raises(KeyError): + store.set_sequences("summon_a_pony", ["Ctrl+Alt+P"]) + + +def test_a_corrupt_file_falls_back_to_defaults(tmp_path) -> None: + """A broken config should not stop the app starting; the cost of ignoring + it is defaults, which are always usable.""" + (tmp_path / "keybindings.json").write_text("{not json", encoding="utf-8") + + store = make_store(tmp_path) + + assert store.sequences_for(shortcuts.NEW_TAB) == shortcuts.sequences_for( + shortcuts.NEW_TAB + ) + + +def test_bindings_for_unknown_actions_are_dropped_on_load(tmp_path) -> None: + """Either a typo or a binding from a newer version - carrying it would let + it collide with something real later.""" + (tmp_path / "keybindings.json").write_text( + json.dumps({"version": 1, "bindings": {"summon_a_pony": ["Ctrl+Alt+P"]}}), + encoding="utf-8", + ) + + store = make_store(tmp_path) + + assert store.conflicts() == {} + assert store.holder_of("Ctrl+Alt+P") is None + + +def test_saving_emits_changed(tmp_path, qtbot) -> None: + store = make_store(tmp_path) + + with qtbot.waitSignal(store.changed, timeout=1000): + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+F"]) + + +def test_the_defaults_have_no_conflicts_through_the_store(tmp_path) -> None: + assert make_store(tmp_path).conflicts() == {} + + +def test_holder_of_names_the_action_using_a_chord(tmp_path) -> None: + store = make_store(tmp_path) + taken = shortcuts.sequences_for(shortcuts.CLOSE_TAB)[0] + + assert store.holder_of(taken) == shortcuts.CLOSE_TAB + assert store.holder_of("Ctrl+Alt+Shift+F12") is None diff --git a/tests/test_keybindings_dialog.py b/tests/test_keybindings_dialog.py new file mode 100644 index 0000000..aeb9153 --- /dev/null +++ b/tests/test_keybindings_dialog.py @@ -0,0 +1,197 @@ +"""The keyboard shortcuts editor.""" + +from __future__ import annotations + +import pytest +from PySide6.QtGui import QKeySequence +from PySide6.QtWidgets import QMessageBox + +from qtxterm import shortcuts +from qtxterm.keybindings import KeybindingStore +from qtxterm.keybindings_dialog import KeybindingsDialog + + +@pytest.fixture +def store(tmp_path) -> KeybindingStore: + return KeybindingStore(path=tmp_path / "keybindings.json") + + +@pytest.fixture +def dialog(qtbot, store) -> KeybindingsDialog: + widget = KeybindingsDialog(store) + qtbot.addWidget(widget) + return widget + + +def select_action(dialog: KeybindingsDialog, action: str) -> None: + from qtxterm.keybindings_dialog import _ACTION_ROLE + + for row in range(dialog._tree.topLevelItemCount()): + item = dialog._tree.topLevelItem(row) + if item.data(0, _ACTION_ROLE) == action: + dialog._tree.setCurrentItem(item) + return + raise AssertionError(f"no row for {action}") + + +def test_every_action_gets_a_row_with_a_readable_name(dialog) -> None: + """A row reading "focus_pane_left" would be nobody's idea of a preference.""" + assert dialog._tree.topLevelItemCount() == len(shortcuts.all_actions()) + + labels = { + dialog._tree.topLevelItem(row).text(0) + for row in range(dialog._tree.topLevelItemCount()) + } + assert "Split right" in labels + assert not any("_" in label for label in labels) + + +def test_only_actions_with_several_chords_get_child_rows(dialog, store) -> None: + """A child per chord, so one can be removed without clearing the lot - + but only where there is a choice to make. A disclosure arrow revealing a + copy of the row above it is noise. + + Which actions have several chords is platform-dependent (macOS splits on + a single Cmd+D where Windows has four spellings), so this asserts the + rule rather than naming an action. + """ + from qtxterm.keybindings_dialog import _ACTION_ROLE + + for row in range(dialog._tree.topLevelItemCount()): + item = dialog._tree.topLevelItem(row) + action = item.data(0, _ACTION_ROLE) + chords = store.sequences_for(action) + expected = len(chords) if len(chords) > 1 else 0 + assert item.childCount() == expected, action + + +def test_an_actions_row_lists_all_of_its_chords(dialog, store) -> None: + """One line per action, so the list stays scannable at two dozen rows.""" + from qtxterm.keybindings_dialog import _ACTION_ROLE + + item = dialog._tree.topLevelItem(0) + action = item.data(0, _ACTION_ROLE) + + assert item.text(1) == ", ".join(store.display_sequences_for(action)) + + +def test_an_action_with_no_chord_says_so(dialog, store) -> None: + from qtxterm.keybindings_dialog import _ACTION_ROLE + + store.set_sequences(shortcuts.FIND, []) + dialog._reload() + + for row in range(dialog._tree.topLevelItemCount()): + item = dialog._tree.topLevelItem(row) + if item.data(0, _ACTION_ROLE) == shortcuts.FIND: + assert item.text(1) == "None" + return + raise AssertionError("no row for find") + + +def test_removing_the_only_chord_works_without_expanding(dialog, store) -> None: + """A single-chord action has no children to select, so Remove has to act + on that chord directly or the button would be permanently dead.""" + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F"]) + dialog._reload() + select_action(dialog, shortcuts.FIND) + + dialog._remove() + + assert store.sequences_for(shortcuts.FIND) == [] + + +def test_adding_a_chord_keeps_the_existing_ones(dialog, store) -> None: + select_action(dialog, shortcuts.FIND) + before = list(store.sequences_for(shortcuts.FIND)) + dialog._capture.setKeySequence(QKeySequence("Ctrl+Alt+Shift+F")) + + dialog._add() + + assert store.sequences_for(shortcuts.FIND) == [*before, "Ctrl+Alt+Shift+F"] + + +def test_removing_a_chord_leaves_the_others(dialog, store) -> None: + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F", "Ctrl+Alt+Shift+G"]) + dialog._reload() + select_action(dialog, shortcuts.FIND) + item = dialog._tree.currentItem() + dialog._tree.setCurrentItem(item.child(0)) + + dialog._remove() + + assert store.sequences_for(shortcuts.FIND) == ["Ctrl+Alt+Shift+G"] + + +def test_a_clash_is_reported_and_nothing_is_changed( + dialog, store, monkeypatch +) -> None: + """Qt fires neither of two shortcuts sharing a chord, so silently taking + the new binding would break both actions.""" + warnings = [] + monkeypatch.setattr( + QMessageBox, "warning", lambda *args, **kwargs: warnings.append(args[2]) + ) + taken = shortcuts.sequences_for(shortcuts.CLOSE_TAB)[0] + select_action(dialog, shortcuts.NEW_TAB) + dialog._capture.setKeySequence(QKeySequence(taken)) + + dialog._add() + + assert warnings, "the clash was not reported" + assert "Close tab" in warnings[0] + assert store.is_customised(shortcuts.NEW_TAB) is False + + +def test_resetting_an_action_restores_its_default(dialog, store) -> None: + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F"]) + dialog._reload() + select_action(dialog, shortcuts.FIND) + + dialog._reset() + + assert store.is_customised(shortcuts.FIND) is False + + +def test_customised_actions_are_shown_in_bold(dialog, store) -> None: + from qtxterm.keybindings_dialog import _ACTION_ROLE + + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F"]) + dialog._reload() + + bold = { + dialog._tree.topLevelItem(row).data(0, _ACTION_ROLE) + for row in range(dialog._tree.topLevelItemCount()) + if dialog._tree.topLevelItem(row).font(0).bold() + } + assert bold == {shortcuts.FIND} + + +def test_reset_all_puts_everything_back(dialog, store, monkeypatch) -> None: + monkeypatch.setattr( + QMessageBox, "question", lambda *a, **k: QMessageBox.StandardButton.Yes + ) + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F"]) + dialog._reload() + + dialog._reset_all() + + assert not any(store.is_customised(a) for a in shortcuts.all_actions()) + + +def test_reset_all_can_be_declined(dialog, store, monkeypatch) -> None: + monkeypatch.setattr( + QMessageBox, "question", lambda *a, **k: QMessageBox.StandardButton.No + ) + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F"]) + dialog._reload() + + dialog._reset_all() + + assert store.is_customised(shortcuts.FIND) is True + + +def test_only_one_chord_is_captured_at_a_time(dialog) -> None: + """Qt records up to four chords in a row by default, and nothing in this + app dispatches a multi-step sequence.""" + assert dialog._capture.maximumSequenceLength() == 1 diff --git a/tests/test_shortcuts.py b/tests/test_shortcuts.py new file mode 100644 index 0000000..9c14290 --- /dev/null +++ b/tests/test_shortcuts.py @@ -0,0 +1,151 @@ +"""The cross-platform shortcut table. + +These are rules rather than a transcription of the table: asserting that +NEW_TAB is "Ctrl+Shift+T" would only restate the source. What is worth +pinning is the reasoning that made each choice, because those are the things +that break silently - a shortcut bound to a chord the OS eats, or two actions +quietly cancelling each other out. +""" + +from __future__ import annotations + +import pytest + +from qtxterm import shortcuts + +# Chords bash and readline own on Windows and Linux. Binding any of these at +# application level would shadow the shell inside every terminal. +SHELL_OWNED_LETTERS = "ACDEFGHKLNPRSTUWZ" + + +@pytest.fixture +def on_mac(monkeypatch): + monkeypatch.setattr(shortcuts, "IS_MAC", True) + + +@pytest.fixture +def on_windows(monkeypatch): + monkeypatch.setattr(shortcuts, "IS_MAC", False) + + +def test_no_sequence_is_bound_to_two_actions_on_windows(on_windows) -> None: + """Two QShortcuts sharing a sequence makes Qt fire *neither* - it reports + the ambiguity and gives up - so a collision disables both actions with + nothing logged.""" + assert shortcuts.conflicts() == {} + + +def test_no_sequence_is_bound_to_two_actions_on_mac(on_mac) -> None: + assert shortcuts.conflicts() == {} + + +def test_every_action_resolves_on_both_platforms(monkeypatch) -> None: + for is_mac in (False, True): + monkeypatch.setattr(shortcuts, "IS_MAC", is_mac) + for action in shortcuts.all_actions(): + assert shortcuts.sequences_for(action), (action, is_mac) + + +def test_windows_never_takes_a_plain_ctrl_letter(on_windows) -> None: + """The whole reason this app uses Ctrl+Shift: on Windows and Linux the + shell owns Ctrl+letter. Ctrl+C interrupts, Ctrl+W deletes a word, Ctrl+D + sends EOF - binding any of them would break line editing in every tab.""" + for action in shortcuts.all_actions(): + for sequence in shortcuts.sequences_for(action): + for letter in SHELL_OWNED_LETTERS: + assert sequence != f"Ctrl+{letter}", (action, sequence) + + +def test_mac_leaves_the_interrupt_alone(on_mac) -> None: + """On macOS Qt's "Ctrl" is Command, so Ctrl+C is copy and is correct. + The interrupt is physical Control+C, which Qt spells Meta+C - and that + must stay unbound or Ctrl+C stops reaching the shell.""" + bound = { + sequence + for action in shortcuts.all_actions() + for sequence in shortcuts.sequences_for(action) + } + + assert "Meta+C" not in bound + assert "Ctrl+C" in shortcuts.sequences_for(shortcuts.COPY) + + +def test_mac_next_tab_avoids_the_application_switcher(on_mac) -> None: + """Qt turns "Ctrl+Tab" into Cmd+Tab on macOS, which the OS takes for its + app switcher. Meta+Tab is the spelling that means a physical Ctrl+Tab.""" + sequences = shortcuts.sequences_for(shortcuts.NEXT_TAB) + + assert "Ctrl+Tab" not in sequences + assert "Meta+Tab" in sequences + + +def test_mac_uses_command_without_shift(on_mac) -> None: + """Command is free on macOS - the shell uses Control - so the native + binding is Cmd+T, not the Cmd+Shift+T that translating the Windows chord + would produce.""" + assert shortcuts.sequences_for(shortcuts.NEW_TAB) == ["Ctrl+T"] + assert shortcuts.sequences_for(shortcuts.CLOSE_TAB) == ["Ctrl+W"] + assert shortcuts.sequences_for(shortcuts.FIND) == ["Ctrl+F"] + + +def test_windows_split_covers_both_spellings_of_each_chord(on_windows) -> None: + """A punctuation chord arrives as a different Qt key depending on the + modifier held, and guessing wrong makes the shortcut silently dead.""" + right = shortcuts.sequences_for(shortcuts.SPLIT_RIGHT) + down = shortcuts.sequences_for(shortcuts.SPLIT_DOWN) + + assert {"Alt+Shift+=", "Alt+Shift++"} <= set(right) + assert {"Ctrl+Shift+|", "Ctrl+Shift+\\"} <= set(right) + assert {"Alt+Shift+-", "Alt+Shift+_"} <= set(down) + assert {"Ctrl+Shift+_", "Ctrl+Shift+-"} <= set(down) + + +def test_zoom_out_does_not_collide_with_split_down(on_windows) -> None: + """Ctrl+Shift+- is split-down. Adding it to zoom-out as well - which + looks harmless, since Ctrl+- and Ctrl+Shift+- feel like one gesture - + would make Qt fire neither.""" + assert set(shortcuts.sequences_for(shortcuts.ZOOM_OUT)).isdisjoint( + shortcuts.sequences_for(shortcuts.SPLIT_DOWN) + ) + + +def test_the_last_tab_slot_means_the_last_tab(monkeypatch) -> None: + """Following browsers and Windows Terminal, so the binding keeps working + once there are more tabs than slots.""" + assert shortcuts.LAST_TAB_SLOT == shortcuts.TAB_SLOTS + assert shortcuts.tab_slot_action(3) in shortcuts.all_actions() + +def test_display_names_are_unchanged_off_mac(on_windows) -> None: + for action in shortcuts.all_actions(): + assert shortcuts.display_sequences_for(action) == shortcuts.sequences_for(action) + + +def test_mac_display_names_use_the_keys_people_actually_press(on_mac) -> None: + """Qt's "Ctrl" is Command on macOS, so a binding Qt spells Ctrl+T is + Cmd+T on the keycap, in the menus and in the guide. Comparing Qt's + spelling against user-facing text is what failed the docs test on macOS + CI while both the code and the guide were correct.""" + assert shortcuts.display_sequences_for(shortcuts.NEW_TAB) == ["Cmd+T"] + assert shortcuts.display_sequences_for(shortcuts.SPLIT_RIGHT) == ["Cmd+D"] + + +def test_mac_display_names_translate_meta_to_control(on_mac) -> None: + """Meta is physical Control on macOS. Ctrl is translated to Cmd first + precisely so this rule cannot rewrite a Ctrl it just produced.""" + shown = shortcuts.display_sequences_for(shortcuts.NEXT_TAB) + + assert "Ctrl+Tab" in shown + assert "Cmd+Tab" not in shown, "Cmd+Tab is the OS app switcher" + + +def test_mac_display_names_translate_alt_to_option(on_mac) -> None: + assert shortcuts.display_sequences_for(shortcuts.FOCUS_PANE_LEFT) == [ + "Cmd+Opt+Left" + ] + + +def test_display_translation_leaves_a_trailing_plus_alone(on_mac) -> None: + """Zoom in is Ctrl and the plus key. Substituting the bare word "Ctrl" + rather than "Ctrl+" would have to reason about which trailing + is a + separator and which is the key.""" + assert "Cmd++" in shortcuts.display_sequences_for(shortcuts.ZOOM_IN) diff --git a/tests/test_terminal_tabs.py b/tests/test_terminal_tabs.py index f21313d..0e92468 100644 --- a/tests/test_terminal_tabs.py +++ b/tests/test_terminal_tabs.py @@ -6,11 +6,14 @@ from conftest import FakePtySession from PySide6.QtCore import QPoint, QSettings, Qt +from PySide6.QtGui import QKeySequence, QShortcut from PySide6.QtWidgets import QSplitter from qtxterm.appearance import Appearance, AppearanceStore from qtxterm import terminal_tabs from qtxterm.pty_backend import default_shell +from qtxterm import shortcuts +from qtxterm.pane import PANE_BORDER_WIDTH from qtxterm.terminal_tabs import TerminalTabWidget from qtxterm.browser_widget import BrowserWidget from qtxterm.terminal_widget import TerminalWidget, shell_short_name @@ -288,6 +291,31 @@ def test_running_a_command_with_a_browser_tab_active_is_a_no_op(qtbot) -> None: assert pty.write_calls == [] +def test_find_opens_in_the_focused_terminal(qtbot) -> None: + tabs = make_tabs(qtbot) + terminal = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + opened = [] + terminal.show_find = lambda: opened.append(True) + + tabs.show_find_in_active() + + assert opened == [True] + + +def test_find_with_a_browser_tab_active_is_a_no_op(qtbot) -> None: + """Same rule as commands: a web page has no scrollback to search, and + quietly searching another terminal would put the bar out of sight.""" + tabs = make_tabs(qtbot) + terminal = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + opened = [] + terminal.show_find = lambda: opened.append(True) + tabs.new_browser_tab(url="about:blank") + + tabs.show_find_in_active() + + assert opened == [] + + def test_closing_a_browser_tab_shuts_it_down_and_renumbers(qtbot) -> None: tabs = make_tabs(qtbot) tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) @@ -855,3 +883,567 @@ def test_run_macro_feeds_each_step_its_own_lines(qtbot) -> None: assert ptys[0].write_calls == ["first\r"] assert ptys[1].write_calls == ["second\r"] + + +def test_a_new_tab_takes_the_keyboard(qtbot, monkeypatch) -> None: + """addTab leaves Qt's focus on the tab bar, so without this a new terminal + needs a click before it accepts a keystroke - and until that click, arrow + keys reach the QTabBar and switch tabs instead.""" + tabs = make_tabs(qtbot) + focused = [] + monkeypatch.setattr(TerminalWidget, "focus_pane", lambda self: focused.append(self)) + + terminal = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + + assert focused == [terminal] + + +def test_splitting_gives_the_keyboard_to_the_new_pane(qtbot, monkeypatch) -> None: + """The reported bug: after a split, focus stayed on the QTabBar, so the + arrow keys switched tabs rather than moving between panes.""" + tabs = make_tabs(qtbot) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + focused = [] + monkeypatch.setattr(TerminalWidget, "focus_pane", lambda self: focused.append(self)) + + new = tabs.split_active(Qt.Orientation.Horizontal, pty_session=FakePtySession()) + + assert focused[-1] is new + + +def test_closing_a_pane_gives_the_keyboard_to_the_survivor(qtbot, monkeypatch) -> None: + tabs = make_tabs(qtbot) + first = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + tabs.split_active(Qt.Orientation.Horizontal, pty_session=FakePtySession()) + focused = [] + monkeypatch.setattr(TerminalWidget, "focus_pane", lambda self: focused.append(self)) + + tabs.close_active_pane() + + assert focused[-1] is first + + +def _nested_layout(qtbot, tabs): + """LEFT beside a stacked TOPRIGHT / BOTRIGHT, laid out for real. + + Shown and resized because this is the one behaviour decided by on-screen + geometry - an unlaid-out tab gives every pane the same rect, and the test + would then be passing on nothing. + """ + tabs.resize(800, 600) + tabs.show() + left = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + top = tabs.split_active(Qt.Orientation.Horizontal, pty_session=FakePtySession()) + bottom = tabs.split_active(Qt.Orientation.Vertical, pty_session=FakePtySession()) + qtbot.waitUntil( + lambda: tabs._pane_rect(left).width() > 0 + and tabs._pane_rect(top).top() < tabs._pane_rect(bottom).top(), + timeout=5000, + ) + return left, top, bottom + + +def test_alt_arrow_moves_between_panes_by_geometry(qtbot) -> None: + tabs = make_tabs(qtbot) + left, top_right, bottom_right = _nested_layout(qtbot, tabs) + + tabs._focused_panes[tabs.currentWidget()] = bottom_right + assert tabs.focus_pane_in_direction(-1, 0) is left + assert tabs.focus_pane_in_direction(1, 0) is top_right + assert tabs.focus_pane_in_direction(0, 1) is bottom_right + assert tabs.focus_pane_in_direction(0, -1) is top_right + + +def test_moving_right_prefers_the_pane_that_lines_up(qtbot) -> None: + """From the tall left pane, the two right-hand panes' centres sat 138 and + 139 pixels off axis - one pixel apart, so ranking by centre distance made + the choice a coin flip that would flip again if the splitter moved. + Overlap is stable under that, and the top-left tie-break settles a tie.""" + tabs = make_tabs(qtbot) + left, top_right, _bottom = _nested_layout(qtbot, tabs) + + tabs._focused_panes[tabs.currentWidget()] = left + + assert tabs.focus_pane_in_direction(1, 0) is top_right + + +def test_navigating_past_the_edge_does_nothing(qtbot) -> None: + """No wraparound: panes are a spatial layout, and jumping from the + rightmost pane back to the leftmost is not what "right" means.""" + tabs = make_tabs(qtbot) + _left, top_right, _bottom = _nested_layout(qtbot, tabs) + + tabs._focused_panes[tabs.currentWidget()] = top_right + + assert tabs.focus_pane_in_direction(0, -1) is None + assert tabs.focus_pane_in_direction(1, 0) is None + + +def test_alt_arrow_does_nothing_in_an_unsplit_tab(qtbot) -> None: + tabs = make_tabs(qtbot) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + + assert tabs.focus_pane_in_direction(1, 0) is None + + +def _shortcut_for(tabs, sequence: str): + """The QShortcut registered for `sequence`, or None.""" + from PySide6.QtGui import QShortcut + + for shortcut in tabs.findChildren(QShortcut): + if shortcut.key().toString() == sequence: + return shortcut + return None + + +def test_split_is_bound_to_the_chord_the_keyboard_actually_sends( + qtbot, monkeypatch +) -> None: + """The reported bug: neither split shortcut fired from a real keyboard. + + Qt matches "Alt+Shift+=" against Key_Equal, but holding Shift and pressing + that key sends Key_Plus - and "Alt+Shift+-" arrives as Key_Underscore. Both + spellings have to be registered or the documented chord does nothing. + """ + # Pinned to the Windows/Linux table, because that is what the test is + # about: the punctuation chords only exist there. macOS splits on Cmd+D, + # where no such ambiguity arises - and asserting these chords on a Mac + # failed CI for a binding that was never meant to be registered. + monkeypatch.setattr(shortcuts, "IS_MAC", False) + tabs = make_tabs(qtbot) + + for sequence in ( + # Alt+Shift fails one way: Qt matches "Alt+Shift+=" against Key_Equal, + # but Shift plus that key sends Key_Plus. + "Alt+Shift+=", + "Alt+Shift++", + "Alt+Shift+-", + "Alt+Shift+_", + # Ctrl+Shift fails the opposite way: with Ctrl held the character is a + # control code (Ctrl+_ is 0x1f), so Qt cannot derive "_" from the + # layout and reports the base key - Key_Minus, or Key_Backslash for + # the bar. + "Ctrl+Shift+|", + "Ctrl+Shift+\\", + "Ctrl+Shift+_", + "Ctrl+Shift+-", + ): + assert _shortcut_for(tabs, sequence) is not None, sequence + + +def test_every_split_chord_actually_splits(qtbot) -> None: + """Activated directly rather than by a key press, so this asserts the + wiring without depending on which window the runner has active.""" + # Taken from the table rather than written out, so this covers whichever + # platform it runs on - the Windows chords do not exist on macOS, and + # hardcoding them failed CI there. + cases = [ + (sequence, Qt.Orientation.Horizontal) + for sequence in shortcuts.sequences_for(shortcuts.SPLIT_RIGHT) + ] + [ + (sequence, Qt.Orientation.Vertical) + for sequence in shortcuts.sequences_for(shortcuts.SPLIT_DOWN) + ] + for sequence, orientation in cases: + tabs = make_tabs(qtbot) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + shortcut = _shortcut_for(tabs, sequence) + assert shortcut is not None, sequence + + shortcut.activated.emit() + + assert len(tabs._panes_in(tabs.currentWidget())) == 2, sequence + splitter = tabs.currentWidget() + assert isinstance(splitter, QSplitter) + assert splitter.orientation() is orientation, sequence + +def test_copy_shortcut_copies_the_active_terminals_selection(qtbot) -> None: + from PySide6.QtGui import QGuiApplication + + tabs = make_tabs(qtbot) + terminal = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + terminal._bridge.setSelection("selected output") + + assert tabs.copy_in_active() is True + assert QGuiApplication.clipboard().text() == "selected output" + + +def test_copy_with_nothing_selected_does_not_clear_the_clipboard(qtbot) -> None: + """Matters most on macOS, where the binding is plain Cmd+C: pressing it + with no selection should do nothing, not wipe what you already copied.""" + from PySide6.QtGui import QGuiApplication + + tabs = make_tabs(qtbot) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + QGuiApplication.clipboard().setText("kept") + + assert tabs.copy_in_active() is False + assert QGuiApplication.clipboard().text() == "kept" + + +def test_paste_goes_through_the_terminal_so_bracketed_paste_is_honoured(qtbot) -> None: + tabs = make_tabs(qtbot) + terminal = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + pasted = [] + terminal.paste_from_clipboard = lambda: pasted.append(True) + + tabs.paste_in_active() + + assert pasted == [True] + + +def test_copy_and_paste_do_nothing_while_a_browser_pane_is_active(qtbot) -> None: + tabs = make_tabs(qtbot) + terminal = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + pasted = [] + terminal.paste_from_clipboard = lambda: pasted.append(True) + tabs.new_browser_tab(url="about:blank") + + tabs.paste_in_active() + + assert pasted == [] + assert tabs.copy_in_active() is False + + +def test_zoom_changes_the_stored_font_size(qtbot, tmp_path: Path) -> None: + """Saved through the store rather than pushed at the open terminals, so + it survives a restart and reaches panes opened later.""" + store = make_appearance_store(tmp_path) + tabs = TerminalTabWidget(appearance_store=store) + qtbot.addWidget(tabs) + start = store.current.font_size + + assert tabs.zoom_font(1) == start + 1 + assert store.current.font_size == start + 1 + assert tabs.zoom_font(-1) == start + assert store.current.font_size == start + + +def test_zoom_stops_at_the_same_bounds_the_preferences_dialog_uses( + qtbot, tmp_path: Path +) -> None: + """Zooming past a size the dialog refuses to show would leave a + preference you could see but not edit back.""" + from qtxterm.appearance import MAX_FONT_SIZE, MIN_FONT_SIZE + + store = make_appearance_store(tmp_path) + tabs = TerminalTabWidget(appearance_store=store) + qtbot.addWidget(tabs) + + for _ in range(200): + tabs.zoom_font(1) + assert store.current.font_size == MAX_FONT_SIZE + + for _ in range(200): + tabs.zoom_font(-1) + assert store.current.font_size == MIN_FONT_SIZE + + +def test_zoom_reset_returns_to_the_default_size(qtbot, tmp_path: Path) -> None: + from qtxterm.appearance import DEFAULT_FONT_SIZE + + store = make_appearance_store(tmp_path) + tabs = TerminalTabWidget(appearance_store=store) + qtbot.addWidget(tabs) + tabs.zoom_font(5) + + assert tabs.reset_font_zoom() == DEFAULT_FONT_SIZE + assert store.current.font_size == DEFAULT_FONT_SIZE + + +def test_zoom_is_a_no_op_without_an_appearance_store(qtbot) -> None: + tabs = make_tabs(qtbot) + + assert tabs.zoom_font(1) is None + assert tabs.reset_font_zoom() is None + + +def test_tab_slots_select_by_position(qtbot) -> None: + tabs = make_tabs(qtbot) + for _ in range(4): + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + + assert tabs.activate_tab_slot(1) is True + assert tabs.currentIndex() == 0 + assert tabs.activate_tab_slot(3) is True + assert tabs.currentIndex() == 2 + + +def test_the_last_slot_means_the_last_tab_not_the_ninth(qtbot) -> None: + """Following browsers and Windows Terminal, so it stays useful with more + tabs than slots - and does not simply fail with fewer.""" + tabs = make_tabs(qtbot) + for _ in range(3): + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + + assert tabs.activate_tab_slot(shortcuts.LAST_TAB_SLOT) is True + assert tabs.currentIndex() == 2 + + +def test_a_slot_past_the_last_tab_does_nothing(qtbot) -> None: + tabs = make_tabs(qtbot) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + tabs.setCurrentIndex(0) + + assert tabs.activate_tab_slot(5) is False + assert tabs.currentIndex() == 0 + + +def test_tab_slots_do_nothing_with_no_tabs_open(qtbot) -> None: + tabs = make_tabs(qtbot) + + assert tabs.activate_tab_slot(1) is False + + +def test_every_shortcut_action_is_registered_on_the_widget(qtbot) -> None: + """Catches an action added to the table but never wired up - which would + look fine everywhere except when you pressed the key.""" + tabs = make_tabs(qtbot) + registered = {s.key().toString() for s in tabs.findChildren(QShortcut)} + + for action in shortcuts.all_actions(): + for sequence in shortcuts.sequences_for(action): + assert QKeySequence(sequence).toString() in registered, (action, sequence) + +def make_exit_store(tmp_path: Path, choice: str): + from qtxterm.exit_prefs import PaneExitStore + + settings = QSettings(str(tmp_path / "exit.ini"), QSettings.Format.IniFormat) + store = PaneExitStore(settings) + store.save(choice) + return store + + +def make_tabs_with_exit(qtbot, tmp_path: Path, choice: str) -> TerminalTabWidget: + tabs = TerminalTabWidget(exit_store=make_exit_store(tmp_path, choice)) + qtbot.addWidget(tabs) + return tabs + + +def test_a_clean_shell_exit_closes_its_pane(qtbot, tmp_path: Path) -> None: + from qtxterm.exit_prefs import CLOSE_CLEAN + + tabs = make_tabs_with_exit(qtbot, tmp_path, CLOSE_CLEAN) + pty = FakePtySession() + tabs.new_tab(shell="/bin/bash", pty_session=pty) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + assert tabs.count() == 2 + + pty.exited.emit(0) + + qtbot.waitUntil(lambda: tabs.count() == 1, timeout=2000) + + +def test_a_failed_shell_keeps_its_pane_so_you_can_read_the_error( + qtbot, tmp_path: Path +) -> None: + from qtxterm.exit_prefs import CLOSE_CLEAN + + tabs = make_tabs_with_exit(qtbot, tmp_path, CLOSE_CLEAN) + pty = FakePtySession() + tabs.new_tab(shell="/bin/bash", pty_session=pty) + + pty.exited.emit(1) + qtbot.wait(200) + + assert tabs.count() == 1 + + +def test_always_closes_even_a_failed_shell(qtbot, tmp_path: Path) -> None: + from qtxterm.exit_prefs import CLOSE_ALWAYS + + tabs = make_tabs_with_exit(qtbot, tmp_path, CLOSE_ALWAYS) + pty = FakePtySession() + tabs.new_tab(shell="/bin/bash", pty_session=pty) + + pty.exited.emit(1) + + qtbot.waitUntil(lambda: tabs.count() == 0, timeout=2000) + + +def test_never_leaves_the_pane_alone(qtbot, tmp_path: Path) -> None: + from qtxterm.exit_prefs import CLOSE_NEVER + + tabs = make_tabs_with_exit(qtbot, tmp_path, CLOSE_NEVER) + pty = FakePtySession() + tabs.new_tab(shell="/bin/bash", pty_session=pty) + + pty.exited.emit(0) + qtbot.wait(200) + + assert tabs.count() == 1 + + +def test_an_exiting_shell_closes_only_its_own_pane_in_a_split( + qtbot, tmp_path: Path +) -> None: + """A shell exits in whichever pane it was running, very often not the one + you are looking at - which is why closing acts on the pane that exited + rather than the focused one.""" + from qtxterm.exit_prefs import CLOSE_CLEAN + + tabs = make_tabs_with_exit(qtbot, tmp_path, CLOSE_CLEAN) + first_pty = FakePtySession() + first = tabs.new_tab(shell="/bin/bash", pty_session=first_pty) + second = tabs.split_active( + Qt.Orientation.Horizontal, pty_session=FakePtySession() + ) + assert len(tabs._panes_in(tabs.currentWidget())) == 2 + + first_pty.exited.emit(0) + + qtbot.waitUntil( + lambda: tabs._panes_in(tabs.currentWidget()) == [second], timeout=2000 + ) + assert first not in tabs._panes_in(tabs.currentWidget()) + + +def test_no_exit_store_means_the_old_behaviour(qtbot) -> None: + """A TerminalTabWidget built without one - as tests and embedders do - + must not start closing panes on its own.""" + tabs = make_tabs(qtbot) + pty = FakePtySession() + tabs.new_tab(shell="/bin/bash", pty_session=pty) + + pty.exited.emit(0) + qtbot.wait(200) + + assert tabs.count() == 1 + + +def test_a_tab_closed_before_the_deferred_close_runs_is_survivable( + qtbot, tmp_path: Path +) -> None: + """Closing is deferred a turn of the event loop, because deleting the + widget that owns the object currently emitting is how you get a crash + instead of a closed pane. That gap means the pane can already be gone.""" + from qtxterm.exit_prefs import CLOSE_CLEAN + + tabs = make_tabs_with_exit(qtbot, tmp_path, CLOSE_CLEAN) + pty = FakePtySession() + tabs.new_tab(shell="/bin/bash", pty_session=pty) + + pty.exited.emit(0) + tabs.close_tab_at(0) + qtbot.wait(200) + + assert tabs.count() == 0 + +def test_panes_in_a_split_get_different_slices_of_the_background(qtbot) -> None: + """The whole point of spanning: each pane is a separate page, so left to + itself every one paints the entire picture and a tab split three ways + shows it three times.""" + tabs = make_tabs(qtbot) + left, top_right, bottom_right = _nested_layout(qtbot, tabs) + pushed = {} + for pane in (left, top_right, bottom_right): + pane.set_background_geometry = ( + lambda x, y, w, h, p=pane: pushed.__setitem__(p, (x, y, w, h)) + ) + + tabs.refresh_background_geometry() + + assert len(pushed) == 3 + origins = {(x, y) for x, y, _w, _h in pushed.values()} + assert len(origins) == 3, origins + # Every pane is told the same tab size - that is what they window into. + sizes = {(w, h) for _x, _y, w, h in pushed.values()} + assert len(sizes) == 1, sizes + # The left pane starts at the tab's left edge; the right ones do not. + assert pushed[left][0] < pushed[top_right][0] + # The stacked pair share a column but not a row. + assert pushed[top_right][0] == pushed[bottom_right][0] + assert pushed[top_right][1] < pushed[bottom_right][1] + + +def test_an_unsplit_pane_spans_its_whole_tab(qtbot) -> None: + tabs = make_tabs(qtbot) + tabs.resize(700, 500) + tabs.show() + pane = tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + pushed = [] + pane.set_background_geometry = lambda x, y, w, h: pushed.append((x, y, w, h)) + + tabs.refresh_background_geometry() + + assert pushed, "no geometry pushed" + x, y, w, h = pushed[-1] + # Not (0, 0): the origin is the *page*, which starts inside the pane's + # border margin. Measuring the pane instead would shift every slice by + # that margin and leave visible seams where panes meet. + assert (x, y) == (PANE_BORDER_WIDTH, PANE_BORDER_WIDTH) + assert w > 0 and h > 0 + +def make_keybinding_store(tmp_path: Path): + from qtxterm.keybindings import KeybindingStore + + return KeybindingStore(path=tmp_path / "keybindings.json") + + +def test_a_custom_binding_is_used_instead_of_the_default(qtbot, tmp_path: Path) -> None: + store = make_keybinding_store(tmp_path) + store.set_sequences(shortcuts.NEW_TAB, ["Ctrl+Alt+Shift+N"]) + tabs = TerminalTabWidget(keybinding_store=store) + qtbot.addWidget(tabs) + + assert _shortcut_for(tabs, "Ctrl+Alt+Shift+N") is not None + default = shortcuts.sequences_for(shortcuts.NEW_TAB)[0] + assert _shortcut_for(tabs, default) is None + + +def test_rebinding_takes_effect_without_a_restart(qtbot, tmp_path: Path) -> None: + store = make_keybinding_store(tmp_path) + tabs = TerminalTabWidget(keybinding_store=store) + qtbot.addWidget(tabs) + default = shortcuts.sequences_for(shortcuts.FIND)[0] + assert _shortcut_for(tabs, default) is not None + + store.set_sequences(shortcuts.FIND, ["Ctrl+Alt+Shift+F"]) + + assert _shortcut_for(tabs, "Ctrl+Alt+Shift+F") is not None + assert _shortcut_for(tabs, default) is None + + +def test_the_old_shortcut_does_not_survive_a_rebind(qtbot, tmp_path: Path) -> None: + """A left-behind QShortcut keeps firing, and once its replacement exists + the two are ambiguous - at which point Qt fires neither.""" + from PySide6.QtGui import QShortcut + + store = make_keybinding_store(tmp_path) + tabs = TerminalTabWidget(keybinding_store=store) + qtbot.addWidget(tabs) + before = len(tabs.findChildren(QShortcut)) + + for i in range(5): + store.set_sequences(shortcuts.FIND, [f"Ctrl+Alt+Shift+F{i + 1}"]) + qtbot.wait(50) + + after = len([s for s in tabs.findChildren(QShortcut) if s.key().toString()]) + assert after == before, f"{before} shortcuts became {after}" + + +def test_an_action_bound_to_nothing_registers_no_shortcut( + qtbot, tmp_path: Path +) -> None: + store = make_keybinding_store(tmp_path) + store.set_sequences(shortcuts.FIND, []) + tabs = TerminalTabWidget(keybinding_store=store) + qtbot.addWidget(tabs) + + default = shortcuts.sequences_for(shortcuts.FIND)[0] + assert _shortcut_for(tabs, default) is None + + +def test_a_rebound_shortcut_still_does_its_job(qtbot, tmp_path: Path) -> None: + """The binding is only half of it - the new chord has to reach the same + slot the default did.""" + store = make_keybinding_store(tmp_path) + store.set_sequences(shortcuts.SPLIT_RIGHT, ["Ctrl+Alt+Shift+R"]) + tabs = TerminalTabWidget(keybinding_store=store) + qtbot.addWidget(tabs) + tabs.new_tab(shell="/bin/bash", pty_session=FakePtySession()) + + _shortcut_for(tabs, "Ctrl+Alt+Shift+R").activated.emit() + + assert len(tabs._panes_in(tabs.currentWidget())) == 2 diff --git a/tests/test_terminal_widget.py b/tests/test_terminal_widget.py index 3c86c61..a8e88f2 100644 --- a/tests/test_terminal_widget.py +++ b/tests/test_terminal_widget.py @@ -268,3 +268,151 @@ def test_changing_scrollback_is_pushed_to_an_open_terminal(qtbot) -> None: widget.apply_appearance(Appearance(scrollback=50)) assert '"scrollback": 50' in pushed[-1] + + +def test_show_find_opens_the_in_page_find_bar(qtbot) -> None: + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + calls = [] + widget._view.page().runJavaScript = lambda script: calls.append(script) + + widget.show_find() + + assert calls == ["window.showFind && window.showFind();"] + + +def test_hide_find_closes_it(qtbot) -> None: + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + calls = [] + widget._view.page().runJavaScript = lambda script: calls.append(script) + + widget.hide_find() + + assert calls == ["window.hideFind && window.hideFind();"] + + +def test_ctrl_clicking_an_http_link_opens_it(qtbot, monkeypatch) -> None: + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + opened = [] + monkeypatch.setattr( + "qtxterm.terminal_widget.QDesktopServices.openUrl", + lambda url: opened.append(url.toString()), + ) + + widget._bridge.openLink("https://example.com/a?b=1") + + assert opened == ["https://example.com/a?b=1"] + + +def test_a_link_with_a_scheme_we_do_not_open_is_refused(qtbot, monkeypatch) -> None: + """The link addon's regex matches only http/https, but it is not a + security boundary - the text it ran against came out of the terminal, + which over SSH means it came from the remote host. QDesktopServices will + launch a registered handler for any scheme, so the check is repeated + where it decides whether anything is launched at all. + """ + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + opened = [] + monkeypatch.setattr( + "qtxterm.terminal_widget.QDesktopServices.openUrl", + lambda url: opened.append(url.toString()), + ) + + for uri in ( + "file:///C:/Windows/System32/calc.exe", + "ms-msdt:/id PCWDiagnostic", + "javascript:alert(1)", + "vbscript:msgbox", + "", + ): + assert widget.open_link(uri) is False, uri + + assert opened == [] + + +def test_an_http_link_reports_that_it_opened(qtbot, monkeypatch) -> None: + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + monkeypatch.setattr( + "qtxterm.terminal_widget.QDesktopServices.openUrl", lambda url: None + ) + + assert widget.open_link("http://localhost:8080/") is True + +def test_background_image_becomes_a_file_url_for_the_page(qtbot, tmp_path) -> None: + """A bare filesystem path will not do: the page is served from file:// + and would resolve it relative to the assets directory.""" + image = tmp_path / "wall.png" + image.write_bytes(b"not really a png, but it exists") + appearance = Appearance(background_image=str(image)) + + url = TerminalWidget._background_image_url(appearance) + + assert url.startswith("file://") + assert url.endswith("wall.png") + + +def test_a_missing_background_image_resolves_to_nothing(qtbot, tmp_path) -> None: + """Deleting the image should leave a normal terminal, not a half-painted + one with a broken url().""" + appearance = Appearance(background_image=str(tmp_path / "gone.png")) + + assert TerminalWidget._background_image_url(appearance) == "" + assert TerminalWidget._background_image_url(Appearance()) == "" + + +def test_background_settings_are_in_the_initial_page_url(qtbot, tmp_path) -> None: + image = tmp_path / "wall.png" + image.write_bytes(b"x") + appearance = Appearance(background_image=str(image), background_opacity=42) + + query = parse_qs(TerminalWidget._terminal_url(appearance).query()) + + assert query["backgroundOpacity"] == ["42"] + assert query["backgroundImage"][0].endswith("wall.png") + + +def test_background_changes_are_pushed_to_an_open_terminal(qtbot, tmp_path) -> None: + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + pushed = [] + widget._view.page().runJavaScript = lambda script: pushed.append(script) + image = tmp_path / "wall.png" + image.write_bytes(b"x") + + widget.apply_appearance( + Appearance(background_image=str(image), background_opacity=15) + ) + + assert '"backgroundOpacity": 15' in pushed[-1] + assert "wall.png" in pushed[-1] + +def test_background_geometry_is_pushed_to_the_page(qtbot) -> None: + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + widget._bridge.loaded() + pushed = [] + widget._view.page().runJavaScript = lambda script: pushed.append(script) + + widget.set_background_geometry(40, 12, 800, 600) + + assert "applyBackgroundGeometry(40, 12, 800, 600)" in pushed[-1] + + +def test_geometry_pushed_before_the_page_loads_is_replayed(qtbot) -> None: + """Dropping it silently left the first pane of a tab painting the whole + background while its neighbours painted their slices.""" + widget = TerminalWidget(pty_session=FakePtySession()) + qtbot.addWidget(widget) + pushed = [] + widget._view.page().runJavaScript = lambda script: pushed.append(script) + + widget.set_background_geometry(7, 9, 500, 400) + assert not any("applyBackgroundGeometry" in s for s in pushed) + + widget._bridge.loaded() + + assert any("applyBackgroundGeometry(7, 9, 500, 400)" in s for s in pushed) diff --git a/tests/test_usage_docs.py b/tests/test_usage_docs.py new file mode 100644 index 0000000..81e33b7 --- /dev/null +++ b/tests/test_usage_docs.py @@ -0,0 +1,110 @@ +"""The usage guide, checked against the app rather than proofread. + +Two kinds of rot this catches. Documented shortcuts drifting from the ones +actually bound is the obvious one. The subtler one is markdown that looks +right in the file and renders wrong in Help -> Usage: the guide is shown +through QTextBrowser.setMarkdown, whose escaping rules are not GitHub's, so +the file being correct is not evidence that the dialog is. +""" + +from __future__ import annotations + +import pathlib + +import pytest + +from qtxterm import shortcuts +from qtxterm.help_dialog import USAGE_PATH, HelpDialog + +# Shortcuts a user would reasonably expect to find in the guide. Pane focus +# and the tab-number slots are covered by prose rather than every literal +# chord, so they are not listed here. +DOCUMENTED_ACTIONS = [ + shortcuts.NEW_TAB, + shortcuts.CLOSE_TAB, + shortcuts.NEXT_TAB, + shortcuts.FIND, + shortcuts.COPY, + shortcuts.PASTE, + shortcuts.ZOOM_IN, + shortcuts.ZOOM_OUT, + shortcuts.ZOOM_RESET, + shortcuts.SPLIT_RIGHT, + shortcuts.SPLIT_DOWN, + shortcuts.CLOSE_PANE, +] + + +@pytest.fixture(scope="module") +def source() -> str: + return USAGE_PATH.read_text(encoding="utf-8") + + +@pytest.fixture +def rendered(qtbot) -> str: + """What the Help -> Usage dialog actually puts on screen.""" + dialog = HelpDialog() + qtbot.addWidget(dialog) + return dialog.browser.toPlainText() + + +@pytest.mark.parametrize("action", DOCUMENTED_ACTIONS) +def test_every_documented_action_is_in_the_guide(action, source) -> None: + """Catches a shortcut changed in the code and left stale in the docs - + which is exactly how the guide came to advertise chords that could not + fire from a keyboard. + + Matched against the *displayed* names, not Qt's. On macOS the two differ: + Qt calls the binding "Ctrl+T" and the guide correctly calls it "Cmd+T", + so comparing Qt's spelling failed this test on macOS CI while both the + code and the guide were right. + """ + shown = shortcuts.display_sequences_for(action) + + assert any(sequence in source for sequence in shown), (action, shown) + + +def test_the_guide_renders_without_escaping_artefacts(rendered) -> None: + r"""A literal pipe ends a markdown table cell, and every way of escaping + it that Qt accepts leaves something on screen: `\|` keeps its backslash + and `|` renders as the raw entity. The pipe shortcuts therefore live + in a list, where no escaping is needed at all. + """ + assert r"\|" not in rendered + assert "&#" not in rendered + + +def test_the_guide_uses_plain_dashes(source) -> None: + """Matching SPEC.md and README.md, which carry none.""" + assert "\u2014" not in source + + +def test_the_split_chords_survive_rendering(rendered) -> None: + """The pipe is the one character this document cannot put in a table.""" + assert "Ctrl+Shift+|" in rendered + assert "Ctrl+Shift+_" in rendered + + +def test_the_guide_lists_a_config_path_for_every_supported_platform(source) -> None: + """The app runs on all three and stores its files via platformdirs, so + naming only two leaves Mac users with nothing to look for.""" + for marker in ( + "%LOCALAPPDATA%", + "~/Library/Application Support/qtxterm", + "~/.config/qtxterm", + ): + assert marker in source, marker + + +def test_the_guide_covers_macos_shortcuts(source) -> None: + """The two platforms differ by more than a find-and-replace, so the + guide has to say so rather than quoting only the Windows chords.""" + assert "macOS" in source + assert "Cmd+T" in source + assert "Cmd+C" in source + + +def test_usage_ships_beside_the_package(source) -> None: + assert USAGE_PATH.is_file() + assert USAGE_PATH.parent.name == "assets" + assert pathlib.Path(USAGE_PATH).suffix == ".md"