From ab76354ccc183ba5913b6f540ac29cca6cff654e Mon Sep 17 00:00:00 2001 From: Ara Adkins Date: Thu, 24 Sep 2026 14:13:48 +0200 Subject: [PATCH] Add the settings screen This commit adds the settings screen for the plugin and wires it into the existing (very small) feature set. This is mostly just preparatory work for what comes next. --- Co-authored-by: Claude --- Makefile | 50 +++++++--- README.md | 4 +- docs/CONTRIBUTING.md | 53 +++++++--- docs/architecture.md | 62 ++++++++++-- docs/features.md | 23 ++--- docs/roadmap.md | 7 +- src/main.ts | 129 ++++++++++++++++++++---- src/settings/defs.ts | 79 +++++++++++++++ src/settings/tab.ts | 133 +++++++++++++++++++++++++ src/suggest/patch.ts | 17 +++- src/suggest/transform.ts | 4 +- styles.css | 50 +++++++++- test/integration/suggest/patch.test.ts | 50 +++++++++- test/unit/settings/defs.test.ts | 88 ++++++++++++++++ 14 files changed, 665 insertions(+), 84 deletions(-) create mode 100644 src/settings/defs.ts create mode 100644 src/settings/tab.ts create mode 100644 test/unit/settings/defs.test.ts diff --git a/Makefile b/Makefile index 9cd884b..ca7be47 100644 --- a/Makefile +++ b/Makefile @@ -195,34 +195,60 @@ install: ## Build and install into the vault at $DEV_VAULT_PATH # for you — deletes the contents of what it points at, silently, reporting nothing. # # It deliberately does not build: you link once and leave `make dev` running, so a fresh checkout -# has no main.js yet and its link dangles until the first build, hence the reminder. An existing -# real directory is never removed here — it is somebody's install, and possibly their settings — -# and deleting one to save a `rm` is not a trade this target gets to make. +# has no main.js yet and its link dangles until the first build, hence the reminder. +# +# `FORCE=1` takes over a destination this would otherwise refuse: a copied install, a whole-folder +# symlink, or links following a different checkout. It stays opt-in because those files are +# somebody's, and it is safe because of what it will not do. No directory is ever removed: a copied +# install loses the plugin's three files by name, which this checkout rebuilds in a second, and a +# whole-folder symlink loses the link itself, never what it points at. `data.json` is the one thing +# in that folder nobody can regenerate, so it is left where it lies — or carried across when the +# folder *was* the link and the settings are therefore sitting in the checkout. A destination that +# is not a plugin directory is refused either way: force is permission to replace this plugin's +# files, not a licence to guess at somebody else's. .PHONY: link -link: ## Symlink this checkout's files into the vault at $DEV_VAULT_PATH (pairs with make dev) +link: ## Symlink this checkout's files into the vault at $DEV_VAULT_PATH (FORCE=1 to take one over) $(vault-guard) @$(vault-dest); \ here=$$(pwd -P); \ case $$($(dest-shape)) in \ copied) \ - echo "make link: '$$dest' holds an installed copy of the plugin, and possibly its data.json." >&2; \ - echo " Remove it yourself once you are sure, then re-run: rm -r '$$dest'" >&2; \ - exit 1; \ + if [ -z "$(FORCE)" ]; then \ + echo "make link: '$$dest' holds an installed copy of the plugin, and possibly its data.json." >&2; \ + echo " 'FORCE=1 make link' replaces the plugin's files with links and leaves data.json alone." >&2; \ + echo " Or remove it yourself once you are sure, then re-run: rm -r '$$dest'" >&2; \ + exit 1; \ + fi; \ + for f in $(PLUGIN_FILES); do rm -f "$$dest/$$f"; done; \ + echo "Took over the copied install in $$dest; anything else there, data.json included, is untouched."; \ ;; \ whole-link) \ - echo "make link: '$$dest' is a symlink to a whole checkout, which this target no longer makes." >&2; \ - echo " 'make unlink' replaces it safely, or remove it with no trailing slash: rm '$$dest'" >&2; \ - echo " 'rm -r $$dest/' would instead delete the contents of the checkout it points at." >&2; \ - exit 1; \ + if [ -z "$(FORCE)" ]; then \ + echo "make link: '$$dest' is a symlink to a whole checkout, which this target no longer makes." >&2; \ + echo " 'FORCE=1 make link' replaces it with a folder of links, carrying data.json across." >&2; \ + echo " 'make unlink' replaces it with a copied build, or remove it with no trailing slash: rm '$$dest'" >&2; \ + echo " 'rm -r $$dest/' would instead delete the contents of the checkout it points at." >&2; \ + exit 1; \ + fi; \ + was=$$($(dest-target)); \ + rm "$$dest"; \ + mkdir -p "$$dest"; \ + if [ -n "$$was" ] && [ -f "$$was/data.json" ]; then \ + cp "$$was/data.json" "$$dest/" || exit 1; \ + echo "Carried data.json across from $$was, so the plugin keeps the settings it had while linked."; \ + fi; \ + echo "Replaced the whole-folder symlink at $$dest; $${was:-what it pointed at} is untouched."; \ ;; \ other) \ echo "make link: '$$dest' exists and is not a plugin directory." >&2; \ + echo " FORCE=1 does not reach this: it replaces this plugin's files, and will not guess at others'." >&2; \ exit 1; \ ;; \ esac; \ target=$$($(dest-target)); \ - if [ -n "$$target" ] && [ "$$target" != "$$here" ]; then \ + if [ -n "$$target" ] && [ "$$target" != "$$here" ] && [ -z "$(FORCE)" ]; then \ echo "make link: '$$dest' links to '$$target', not this checkout." >&2; \ + echo " 'FORCE=1 make link' re-points them here, deleting nothing: a symlink is replaced, not followed." >&2; \ echo " Its contents are symlinks, so removing it reaches no checkout: rm -r '$$dest'" >&2; \ exit 1; \ fi; \ diff --git a/README.md b/README.md index 8958d18..fde5215 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,11 @@ # Fluidity Fluidity is a plugin for [Obsidian](https://obsidian.md) that finishes the links the link completer -starts while giving you control over exactly how itt's done! +starts while giving you control over exactly how it's done! Obsidian's autocomplete is usually very good at working out _which note you meant_, and then hands you a link that is _nearly_ right. The display text might be capitalized when the sentence wanted it -lowercase, or the linked pointing at the top of a note when you meant it to point at a section +lowercase, or the link pointing at the top of a note when you meant it to point at a section half-way down. Both leave you editing a link that could have just been right the first time. Fluidity changes what gets inserted, at the moment it gets inserted, so there is nothing to go back diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 6e32776..3413123 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -31,17 +31,18 @@ shell, so `make check` works from a bare terminal too while being a little slowe `make help` lists every target, but the main ones you will use are these: -| Target | What it does | -| ---------------- | ----------------------------------------------------------------------- | -| `make build` | typecheck + bundle — a release `main.js` | -| `make dev` | rebuild `main.js` on change, with sourcemaps | -| `make install` | build, then copy the plugin into `$DEV_VAULT_PATH` | -| `make link` | symlink this checkout's files into `$DEV_VAULT_PATH` instead of copying | -| `make unlink` | swap those symlinks back for a copied build | -| `make check` | **everything CI checks**: format, typecheck, lint, all tests | -| `make test-unit` | the pure tests only — fast | -| `make format` | reformat Markdown, JSON, CSS and TypeScript with dprint | -| `make clean` | drop build output, keep `node_modules` | +| Target | What it Does | +| ------------------- | ----------------------------------------------------------------------- | +| `make build` | typecheck + bundle — a release `main.js` | +| `make dev` | rebuild `main.js` on change, with sourcemaps | +| `make install` | build, then copy the plugin into `$DEV_VAULT_PATH` | +| `make link` | symlink this checkout's files into `$DEV_VAULT_PATH` instead of copying | +| `FORCE=1 make link` | the same, taking over an install or a link to another checkout | +| `make unlink` | swap those symlinks back for a copied build | +| `make check` | **everything CI checks**: format, typecheck, lint, all tests | +| `make test-unit` | the pure tests only — fast | +| `make format` | reformat Markdown, JSON, CSS and TypeScript with dprint | +| `make clean` | drop build output, keep `node_modules` | Building without Nix is possible as the toolchain is only Node, and `npm ci && npm run build` is exactly what Obsidian's plugin review runs, so CI checks that path on every push. You will want @@ -80,12 +81,22 @@ make link It takes the same two guards as `make install` and deliberately does not build, since the intent is that you link once and leave `make dev` running — so a fresh checkout has no `main.js` yet, its link -dangles, and the target says so rather than leaving you with a plugin Obsidian cannot load. It never -removes what is already at the destination: if `make install` has put a copied folder there, -`make link` tells you to delete it yourself, because that folder may hold your `data.json`. Settings +dangles, and the target says so rather than leaving you with a plugin Obsidian cannot load. Settings Obsidian writes land in the vault folder beside the links. Reloading is still on you, as Obsidian does not watch the files for changes. +By default it refuses a destination that is already occupied, because those files are somebody's. +`FORCE=1 make link` takes one over, but will never: + +- **Remove the plugin directory.** A copied install loses the plugin's three files _by name_, which + this checkout rebuilds in a second. Anything else in the folder stays. +- **Follow a symlink.** A whole-folder link is removed with no trailing slash, so the checkout on + the other end is untouched. +- **Remove `data.json`.** It is the one thing in that folder nobody can regenerate, so it is left + where it lies. +- **Operate on a unrecognized destination.** Force is permission to replace this plugin's files, not + a licence to guess at somebody else's. + The plugin folder is a real directory and only its contents are links, which is what makes it safe to remove. `rm` deletes a symlink rather than following it, so clearing the folder out costs three links that `make link` rebuilds in a second. A folder that is _itself_ one symlink does not have @@ -129,6 +140,20 @@ Any change to what gets inserted should be exercised against at least this much: 8. **Disabling the plugin**, after which the completer must behave as stock without any intervention from the monkey patch. +Any change to settings should be exercised against this much: + +1. The **status line**, which must read **Active** in green behind a checkmark on a working vault, + and **Inactive** in red behind a crossed octagon the moment the master toggle is turned off. A + missing icon means Obsidian's Lucide knows neither name `settings/tab` tries for it. +2. The **master toggle off**, after which a fluent note completes exactly as it does with the plugin + disabled — and **on again**, after which it adjusts once more without a reload. +3. A **renamed property**, which must take effect on the very next completion with no reload: the + note carrying the old property stops being adjusted, and one carrying the new one starts. +4. **Clearing the property field**, which means `fluent` and shows it as a placeholder. +5. **Reopening the tab**, and restarting Obsidian, after which both settings read back as they were + left. +6. Searching Obsidian's own **settings search** for `fluent`, which must find both settings. + ## Tests The tests are split across two suites, and the split is crucial to our testing strategy: diff --git a/docs/architecture.md b/docs/architecture.md index 652201c..0a10ad0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -54,14 +54,14 @@ settings tab. It makes no decisions of its own, which is what lets everything be without it. ``` -src/main.ts lifecycle only — load settings, install the patch, add the settings tab -src/settings/defs.ts settings interface, defaults, normalization (pure) -src/settings/tab.ts the settings tab +src/main.ts lifecycle only — load settings, add the tab, install/remove the patch +src/settings/defs.ts settings record, defaults, normalization of what was stored (pure) +src/settings/tab.ts the settings tab, declared for Obsidian 1.13 to render src/suggest/patch.ts locate + patch the built-in suggester (the only internals-touching file) src/suggest/item.ts the suggestion-item union and its type guards -src/suggest/transform.ts (item, context, settings) → item : the decision, thin and delegating +src/suggest/transform.ts (item, context, options) → item : the decision, thin and delegating src/fluent/frontmatter.ts read fluency from a frontmatter-shaped object (pure) -src/fluent/display.ts (displayText, isFluent, atSentenceStart) → display text (pure) +src/fluent/display.ts (displayText, atSentenceStart) → display text (pure) src/prose/sentence.ts is this offset a sentence start? (pure) ``` @@ -160,8 +160,9 @@ Obsidian's startup is one nobody can uninstall from inside Obsidian. ### Patching the Instance's Own Prototype The patch goes on `builtin.constructor.prototype`, via -[`monkey-around`](https://github.com/pjeby/monkey-around), and the uninstaller it returns is handed -to `plugin.register()` so that disabling the plugin puts everything back. +[`monkey-around`](https://github.com/pjeby/monkey-around), and the uninstaller it returns is called +from `onunload`, so that disabling the plugin puts everything back — and from the master toggle, so +that switching the feature off does too. It must not go on `EditorSuggest.prototype`. That is shared with the tag suggester, the footnote suggester, and every suggester every other plugin has registered — patching it would have Fluidity @@ -277,6 +278,53 @@ production-grade, it depends on nothing undocumented, and knowing it exists is w current approach a considered choice rather than the only one anybody thought of. The [roadmap](./roadmap.md) tracks it. +## Settings + +Two settings, one read-only status line, and three decisions worth writing down. + +### The Master Toggle Removes the Patch + +Turning fluent titles off uninstalls the wrapper rather than making it inert. Both would leave +completions unchanged, so the difference only matters for the reason somebody reaches for the +switch: Fluidity's risk is that it patches a part of Obsidian that is not meant to be +user-accessible, and an off switch that leaves the patch in place does not retire that risk. Off +means the app is running the code it would run without Fluidity installed. + +`main.ts` owns this. It keeps the current `PatchResult` and reconciles it against the setting; +`suggest/transform` is deliberately given a narrower record than `settings/defs` holds, so that the +decision cannot start answering a question that belongs to the lifecycle. + +One consequence is free: toggling off and back on reinstalls, which is the easy retry for a failed +install. Retrying on every settings change instead would relog the same failure on every keystroke. + +### The Property is Read Per Completion + +`installFluentTitles` takes a **function** returning its options rather than a record, and calls it +inside the wrapper. That is what lets a rename take effect on the next completion. + +### `data.json` is Not Trusted + +Obsidian hands back whatever `JSON.parse` made of a file the user can open and edit, and which may +have been written by an older version of this plugin. The conventional +`Object.assign({}, DEFAULTS, await loadData())` accepts all of it: a `fluentProperty` of `null` +survives and is then used to read frontmatter under the key `"null"`, and a `fluentTitles` of +`"false"` is a truthy string that turns the feature on for somebody whose file says it is off. + +A file that can be edited can also stop being JSON, and that is a separate failure: it happens +before any of this, in `loadData`. `main.ts` catches it, reports one line and starts on the +defaults, because `onload` is the one method here that must not fail: a plugin that breaks +Obsidian's startup is one nobody can disable from inside Obsidian. + +Reading a file makes `onload` asynchronous, and Obsidian is free to unload a plugin while an `await` +inside one is still pending. That is why `main.ts` checks whether it has been unloaded before it +installs anything: a patch applied after the unload that would have removed it stays on the +completer until the app restarts, with nothing left running that knows it is there. + +So `settings/defs` **rebuilds** the record field by field instead of merging, and each field that +does not hold its declared type falls back to its own default. Field by field rather than wholesale, +because the two settings are independent and one unusable value should not silently revert the +other. + ## The Build `esbuild` bundles `src/main.ts` to a single CommonJS `main.js`. The plugin is `main.js`, diff --git a/docs/features.md b/docs/features.md index af7e6c6..dd58180 100644 --- a/docs/features.md +++ b/docs/features.md @@ -210,10 +210,6 @@ See the [roadmap](./roadmap.md). ## Settings -**Not implemented yet.** There is no settings tab. The property is fixed as `fluent` in -`src/main.ts`, there is no master toggle, and changing either takes an edit and a rebuild. What this -section describes is the intended shape, and the [roadmap](./roadmap.md) tracks it. - **Settings → Fluidity**. | Setting | Default | What it does | @@ -221,15 +217,12 @@ section describes is the intended shape, and the [roadmap](./roadmap.md) tracks | **Fluent titles** | on | The master toggle. Off, nothing about a completion is changed. | | **Fluent property** | `fluent` | Which frontmatter property marks a note fluent. | -Renaming the property takes effect immediately and does not migrate anything — notes still carrying -the old property simply stop being treated as fluent. It exists for vaults where `fluent` already -means something else. - -Above these sits a **read-only status line** reporting whether the completer patch installed. It is -the first thing to check when nothing seems to be happening: Fluidity works by patching a part of -Obsidian that is not public API, and an Obsidian update is capable of moving what it attaches to. If -that happens the plugin declines to install the patch, says so here and once in the developer -console, and leaves the completer behaving exactly as it does without the plugin. +Turning **fluent titles** off does not leave a patched completer sitting idle but instead removes +the patch outright, so that off means Obsidian is running the code it would run without Fluidity +installed. -Until that line exists, the developer console is the only place the failure is reported, which is -why checking the plugin by hand starts by opening it. +Renaming the **property** takes effect on the next completion and does not migrate anything; notes +still carrying the old property simply stop being treated as fluent. It exists for vaults where +`fluent` already means something else. Leaving the field empty means `fluent`, which is what the +greyed-out placeholder is telling you; surrounding spaces are dropped, because a trailing one is +invisible in both this field and the property editor and would read as the plugin being broken. diff --git a/docs/roadmap.md b/docs/roadmap.md index a40dd6a..6277c78 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -7,9 +7,10 @@ Nothing here is committed to or has a date; it is a statement of intent. - **Section-Aware Alias Links.** The second half of the plugin, described below. It is the reason Fluidity exists as much as fluent titles are. -- **Real-Vault Coverage of the Settings Tab.** The settings path is exercised by type-checking and - the pure defaults test, not by anything that renders it. Every settings change needs a manual pass - until that is no longer true. +- **Real-Vault Coverage of the Settings Tab.** The normalizer is unit-tested and the tab is + type-checked, but nothing renders it outside a vault — and nothing can, since the controls are + declarations handed to Obsidian to draw. Every settings change needs a manual pass, which the + [contributing guide](./CONTRIBUTING.md#what-to-check-by-hand) lists. - **Mobile.** The manifest says the plugin is not desktop-only, and nothing in the design should care — but the completer is reached differently on the mobile toolbar's `[[` button, and that has not been exercised. Until it has, "should work" is all that can honestly be claimed. diff --git a/src/main.ts b/src/main.ts index 6534707..6ad6e10 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,31 +1,126 @@ /** * Fluidity's entry point. * - * This module owns the plugin lifecycle: installing the completer patch and handing its uninstaller - * to `register()`, so that disabling the plugin puts Obsidian back as it was. Every decision - * belongs to the modules beneath it, which is what lets them be tested without an editor. + * This module owns the plugin lifecycle: loading the settings, installing the completer patch and + * taking it back off again, and registering the settings tab. Every decision belongs to the + * modules beneath it, which is what lets them be tested without an editor. * - * The fluent property is fixed here pending `settings/defs`, which will carry it along with the - * master toggle and the status line that reports what `onload` found. + * It is also the one place that knows the master toggle exists. Turning fluent titles off removes + * the patch rather than making it inert, so that "off" means Obsidian is running the code it would + * run without Fluidity installed — which is the only version of off worth offering for a plugin + * whose whole mechanism is reaching past the public API. */ import { Plugin } from "obsidian"; -import { installFluentTitles } from "./suggest/patch"; - -/** The frontmatter property that marks a note fluent. */ -const FLUENT_PROPERTY = "fluent"; +import { DEFAULT_SETTINGS, type FluiditySettings, normalizeSettings } from "./settings/defs"; +import { FluiditySettingTab } from "./settings/tab"; +import { installFluentTitles, type PatchResult } from "./suggest/patch"; export default class FluidityPlugin extends Plugin { - override onload(): void { - const result = installFluentTitles(this.app, { property: FLUENT_PROPERTY }); - if (result.installed) { - this.register(result.uninstall); - return; + /** Read by the settings tab, and by the patch on every completion. */ + settings: FluiditySettings = { ...DEFAULT_SETTINGS }; + + /** The completer patch, or `null` while fluent titles are off. */ + private patch: PatchResult | null = null; + + /** Whether Obsidian has taken the plugin down, which it may do mid-`onload`. */ + private unloaded = false; + + override async onload(): Promise { + this.settings = normalizeSettings(await this.storedSettings()); + + // Reading the settings puts an `await` in `onload`, and Obsidian is free to unload a plugin + // while one is pending. A patch installed past that point would never come off, because the + // unload that would have removed it has already been and gone. + if (this.unloaded) return; + + this.applySettings(); + + // Added last as Obsidian asks a tab for its settings as soon as it is registered, and a status + // line built before the patch was attempted would report a state nothing had reached. + this.addSettingTab(new FluiditySettingTab(this.app, this)); + } + + /** + * Put the completer back exactly as it was. + * + * The patch is taken off here rather than handed to `register()` at install time, because the + * master toggle installs and removes it repeatedly within one session. There is no single + * uninstaller to register, and registering each one as it is created would stack a callback per + * toggle. + */ + override onunload(): void { + this.unloaded = true; + this.removePatch(); + } + + /** Apply a change from the settings tab, act on it, and record it. */ + async updateSettings(change: Partial): Promise { + this.settings = { ...this.settings, ...change }; + + // Acted on before it is written, so that a save which fails leaves the plugin behaving the way + // the settings tab says it does. Losing the record of a change is recoverable by making it + // again; a toggle that reads off while the patch is still installed is not visible at all. + this.applySettings(); + await this.saveData(this.settings); + } + + /** + * Why the completer is not patched, or `null` if it is. + * + * The settings tab words this; the fact is all that is reported here. While fluent titles are + * off there is no failure to report, because nothing was attempted. + */ + patchFailure(): string | null { + return this.patch !== null && !this.patch.installed ? this.patch.reason : null; + } + + /** + * What is on disk, or nothing if it cannot be read. + * + * `data.json` is a file a user can open and edit, so it can hold text that is not JSON at all — + * and `onload` is the one method in this plugin that must not fail, because a plugin that breaks + * Obsidian's startup is one nobody can disable from inside Obsidian. A file that cannot be parsed + * is reported once and treated as absent, which starts the plugin on its defaults rather than not + * at all. What a parsed file *contains* is `settings/defs`'s problem, and it trusts none of it. + */ + private async storedSettings(): Promise { + try { + return await this.loadData(); + } catch (error) { + console.error("Fluidity: settings could not be read, starting from the defaults", error); + return null; + } + } + + /** Bring the patch into line with the master toggle. */ + private applySettings(): void { + if (this.settings.fluentTitles) this.addPatch(); + else this.removePatch(); + } + + private addPatch(): void { + // A result that is already recorded is left alone. Retrying a failed install on every keystroke + // in the property field would log the same line over and over, for a reason that cannot have + // changed; toggling off and on is how a retry is asked for. + if (this.patch !== null) return; + + // The settings are passed as a function so that renaming the property takes effect on the next + // completion rather than on the next reload. + const result = installFluentTitles(this.app, () => ({ property: this.settings.fluentProperty })); + this.patch = result; + + if (!result.installed) { + // One line, naming the plugin: the completer keeps behaving exactly as it does without + // Fluidity installed. The settings tab reports this where a user can see it, and says it + // without borrowing the master toggle's word for a state the user did not ask for. + console.error(`Fluidity: ${result.reason} — completions are unchanged`); } + } - // One line, naming the plugin: the completer keeps behaving exactly as it does without Fluidity - // installed. The settings tab will report this where a user can see it. - console.error(`Fluidity: ${result.reason} — fluent titles are off, completions are unchanged`); + private removePatch(): void { + if (this.patch?.installed === true) this.patch.uninstall(); + this.patch = null; } } diff --git a/src/settings/defs.ts b/src/settings/defs.ts new file mode 100644 index 0000000..1da91b8 --- /dev/null +++ b/src/settings/defs.ts @@ -0,0 +1,79 @@ +/** + * What Fluidity remembers between sessions, and what to believe when it reads it back. + * + * Obsidian persists a plugin's settings as `data.json` beside the plugin, and hands them back as + * whatever `JSON.parse` made of that file. It is not a trustworthy shape: it is a file a user can + * open and edit, it may have been written by an older version of this plugin, and it may be absent + * entirely on a first run. The usual `Object.assign({}, DEFAULTS, await loadData())` accepts every + * one of those without comment. + * + * So the record is not merged, it is **rebuilt**: every field is checked, and one that does not + * hold the type it is declared with falls back to its default. That makes a corrupt file degrade + * to stock behavior rather than to something strange. This is abnormal for a plugin, but important + * given that fluidity reaches into Obsidian's internals. + * + * It imports nothing, which is what lets `settings/tab` render it and `main` persist it without + * either of them being needed to test the rules. + */ + +/** Everything about Fluidity that a user can change. */ +export interface FluiditySettings { + /** + * The master toggle. + * + * Off means the completer is not patched at all, rather than patched and inert — Fluidity's one + * real risk is reaching past the public API, and an off switch that leaves the wrapper in place + * does not retire that risk for whoever reached for it. + */ + fluentTitles: boolean; + + /** + * The frontmatter property that marks a note fluent. + * + * Renaming it migrates nothing: notes still carrying the old property simply stop being treated + * as fluent. It exists for vaults where `fluent` already means something else. + */ + fluentProperty: string; +} + +/** What Fluidity does before anybody has told it otherwise. */ +export const DEFAULT_SETTINGS: FluiditySettings = { + fluentTitles: true, + fluentProperty: "fluent", +}; + +/** + * Build a settings record from whatever was stored, falling back per field. + * + * Per **field** rather than per record, because the two settings are independent: a `data.json` + * that has been hand-edited into an unusable `fluentProperty` should not also silently flip the + * master toggle back on. + */ +export function normalizeSettings(stored: unknown): FluiditySettings { + const raw: Record = typeof stored === "object" && stored !== null + ? stored as Record + : {}; + + return { + fluentTitles: typeof raw.fluentTitles === "boolean" ? raw.fluentTitles : DEFAULT_SETTINGS.fluentTitles, + fluentProperty: normalizeProperty(raw.fluentProperty), + }; +} + +/** + * The property name to actually look for, given what was typed or stored. + * + * Surrounding whitespace is dropped because it is invisible in both the settings field and the + * property editor, so a stray space would read as the feature being broken rather than as a typo. + * Nothing else is corrected: a property name is the user's own vocabulary, and the only wrong + * answer is one that cannot name a property at all. + * + * It is exported because the settings tab normalizes each keystroke through it, which is what lets + * an empty field mean "the default" rather than "match a property with no name". + */ +export function normalizeProperty(value: unknown): string { + if (typeof value !== "string") return DEFAULT_SETTINGS.fluentProperty; + + const trimmed = value.trim(); + return trimmed === "" ? DEFAULT_SETTINGS.fluentProperty : trimmed; +} diff --git a/src/settings/tab.ts b/src/settings/tab.ts new file mode 100644 index 0000000..4f0f665 --- /dev/null +++ b/src/settings/tab.ts @@ -0,0 +1,133 @@ +/** + * The settings tab: the two controls, and the line that says whether the feature is running. + */ + +import { type App, type IconName, PluginSettingTab, setIcon, type SettingDefinitionItem } from "obsidian"; + +import type FluidityPlugin from "../main"; +import { DEFAULT_SETTINGS, type FluiditySettings, normalizeProperty } from "./defs"; + +/** + * The mark drawn before the status word, each as a list of names to try in order. + * + * `IconName` is an alias for `string`, so nothing here is checked at build time — and Lucide + * renamed the crossed octagon from `x-octagon` to `octagon-x`, which leaves the right name + * depending on the Lucide the running Obsidian bundles. `setIcon` neither throws nor reports on a + * name it does not have; it leaves the element empty, so the only way to ask is to look afterwards. + */ +const ACTIVE_ICONS: IconName[] = ["check"]; +const INACTIVE_ICONS: IconName[] = ["octagon-x", "x-octagon"]; + +export class FluiditySettingTab extends PluginSettingTab { + private readonly plugin: FluidityPlugin; + + constructor(app: App, plugin: FluidityPlugin) { + super(app, plugin); + this.plugin = plugin; + } + + override getSettingDefinitions(): SettingDefinitionItem[] { + // `satisfies` rather than a plain return, so that every `key` below is checked against the + // settings record. Without it a mistyped key is a control that silently reads and writes a + // field nobody has, which looks exactly like a setting that does not save. + return [ + { + name: "Status", + desc: this.status(), + // It answers "why is nothing happening", which is not a question anyone searches for by + // name, and it is not a setting. + searchable: false, + }, + { + name: "Fluent titles", + desc: "Lowercase a fluent note's display text when its link lands mid-sentence.", + aliases: ["lowercase", "capitalization", "sentence case"], + control: { + type: "toggle", + key: "fluentTitles", + defaultValue: DEFAULT_SETTINGS.fluentTitles, + }, + }, + { + name: "Fluent property", + desc: "Which frontmatter property marks a note fluent. Renaming it migrates nothing: notes still carrying" + + " the old property stop being treated as fluent.", + aliases: ["frontmatter", "property"], + control: { + type: "text", + key: "fluentProperty", + // An empty field means the default, which the placeholder is showing. There is no + // `validate` here because there is nothing to reject: every string that can name a + // property is accepted, and one that cannot is read as asking for the default. + placeholder: DEFAULT_SETTINGS.fluentProperty, + defaultValue: DEFAULT_SETTINGS.fluentProperty, + }, + }, + ] satisfies SettingDefinitionItem[]; + } + + /** + * Persist a changed control, and let the plugin act on it. + * + * Only the setter is overridden. The inherited reader already reports `plugin.settings`, which is + * the record these keys name; the inherited writer would assign into it and save, which persists + * the master toggle without ever installing or removing the patch it controls. + */ + override async setControlValue(key: string, value: unknown): Promise { + if (key === "fluentTitles") { + await this.plugin.updateSettings({ fluentTitles: value === true }); + + // Redrawn because the status line above states what this toggle just did, and a settings tab + // reporting the previous answer is worse than one reporting none. It is deliberately not done + // for the property field: rebuilding the rows under a cursor would interrupt typing. + this.update(); + return; + } + + if (key === "fluentProperty") { + await this.plugin.updateSettings({ fluentProperty: normalizeProperty(value) }); + } + } + + /** Say whether completions are being adjusted, and if not, why not. */ + private status(): DocumentFragment { + if (!this.plugin.settings.fluentTitles) { + return statusLine(false, "fluent titles are turned off."); + } + + const failure = this.plugin.patchFailure(); + if (failure === null) { + return statusLine(true, "the link completer is adjusting fluent notes."); + } + + // Named as Fluidity's own failure rather than Obsidian's, and paired with what still works: the + // completer is untouched, so nothing a user does next is at risk. + return statusLine(false, `Fluidity could not start: ${failure}. Completions are unchanged.`); + } +} + +/** + * One word and a mark, then the sentence that says why. + * + * Only the word and its mark carry a color, because they are the part read at a glance; the + * sentence after them is ordinary description text, and coloring a whole line red would make the + * reason harder to read rather than easier to notice. + */ +function statusLine(active: boolean, detail: string): DocumentFragment { + return createFragment((el) => { + const mark = el.createSpan({ cls: ["fluidity-status", active ? "mod-active" : "mod-inactive"] }); + + setFirstIcon(mark.createSpan({ cls: "fluidity-status-icon" }), active ? ACTIVE_ICONS : INACTIVE_ICONS); + mark.createSpan({ text: active ? "Active" : "Inactive" }); + + el.createSpan({ text: ` — ${detail}` }); + }); +} + +/** Draw the first of these icons the running Obsidian actually has, if it has any of them. */ +function setFirstIcon(el: HTMLElement, names: IconName[]): void { + for (const name of names) { + setIcon(el, name); + if (el.firstElementChild !== null) return; + } +} diff --git a/src/suggest/patch.ts b/src/suggest/patch.ts index 7d519bf..dbca896 100644 --- a/src/suggest/patch.ts +++ b/src/suggest/patch.ts @@ -1,7 +1,8 @@ /** * Finding Obsidian's link completer and wrapping the method that inserts a choice. * - * This is the only module that touches Obsidian's internals, and it is deliberately the smallest + * This is the only module that touches Obsidian's internals, and it is deliberately as small as + * possible * one that can be: it locates an object the app never exposes, wraps a single method, captures the * one piece of state that method destroys, and delegates. It makes no decisions — `suggest/item` * says what may be touched and `suggest/transform` says what it becomes. @@ -55,10 +56,16 @@ export type PatchResult = /** * Wrap the completer's `selectSuggestion` so that a fluent note's link reads as prose. * - * The returned uninstaller belongs in `plugin.register()`, so that disabling Fluidity puts the - * completer back exactly as it was. + * The returned uninstaller is the caller's to hold onto. Fluidity's plugin calls it on unload, and + * again whenever the master toggle is switched off, so that either one puts the completer back + * exactly as it was. + * + * `options` is a function rather than a record because the settings tab can change the property + * name between one completion and the next, and reading it per call is what makes that take effect + * immediately. Reinstalling on each change would do the same job while moving Fluidity's wrapper + * to the outside of every other plugin's. */ -export function installFluentTitles(app: App, options: TransformOptions): PatchResult { +export function installFluentTitles(app: App, options: () => TransformOptions): PatchResult { try { const builtin = findLinkSuggest(app); if (builtin === null) { @@ -88,7 +95,7 @@ export function installFluentTitles(app: App, options: TransformOptions): PatchR let chosen = item; try { - chosen = transformSuggestion(app, item, context, options); + chosen = transformSuggestion(app, item, context, options()); } catch (error) { // A failure to adjust is not a reason to swallow the user's keystroke: fall through // with what Obsidian handed us, which inserts exactly what it would have without the diff --git a/src/suggest/transform.ts b/src/suggest/transform.ts index a851ee9..b81f4ed 100644 --- a/src/suggest/transform.ts +++ b/src/suggest/transform.ts @@ -21,7 +21,9 @@ import { isFluent } from "../fluent/frontmatter"; import { startsSentence } from "../prose/sentence"; import { isAliasSuggestion, isFileSuggestion, type LinkSuggestion } from "./item"; -/** What the rule needs to know from the user, pending the settings tab. */ +/** + * What the rule needs to know from the user. + */ export interface TransformOptions { /** The frontmatter property that marks a note fluent. */ property: string; diff --git a/styles.css b/styles.css index 6b365ba..fb6c9e4 100644 --- a/styles.css +++ b/styles.css @@ -1,7 +1,49 @@ /* Fluidity plugin styles. * - * Deliberately near-empty. Fluidity has no views and no widgets — it changes what the built-in - * link completer inserts, and every pixel it touches is Obsidian's own. The file exists because - * `styles.css` is one of the three files Obsidian installs alongside a plugin, so shipping it - * keeps `make install` and the release workflow uniform with every other plugin. + * Deliberately near-empty. Fluidity has no views of its own: it changes what the built-in link + * completer inserts, and its settings tab is assembled from Obsidian's own components, so nearly + * every pixel it touches is styled by the app and by whatever theme is in use. The rules below + * exist because one line of that tab has no native styling to inherit. + * + * The file would ship even with nothing in it — `styles.css` is one of the three files Obsidian + * installs alongside a plugin, and shipping it keeps `make install` and the release workflow + * uniform with every other plugin. + */ + +/* The settings tab's status line. + * + * Obsidian has no variant of `Setting` that reads as a state, so the one word that answers "is this + * running?" is marked here: a green check for active, a red crossed octagon for not. A failure + * reported in the same grey as every other description is one nobody notices, which would defeat + * the point of reporting it outside the developer console at all. + * + * Only the word and its icon are colored. The sentence after them explains which kind of inactive + * this is, and it stays ordinary description text, because a whole line of red is harder to read + * rather than easier to notice. */ +.fluidity-status { + display: inline-flex; + align-items: center; + gap: var(--size-2-1); + font-weight: var(--font-semibold); +} + +.fluidity-status.mod-active { + color: var(--text-success); +} + +.fluidity-status.mod-inactive { + color: var(--text-error); +} + +/* The icon takes its color from the word beside it, as every Lucide icon is drawn in + * `currentColor`. It is sized in `em` rather than from Obsidian's icon scale so that it matches + * whatever size a theme gives description text, instead of towering over a small one. */ +.fluidity-status-icon { + display: inline-flex; +} + +.fluidity-status-icon .svg-icon { + width: 1em; + height: 1em; +} diff --git a/test/integration/suggest/patch.test.ts b/test/integration/suggest/patch.test.ts index e9aed83..6ac9866 100644 --- a/test/integration/suggest/patch.test.ts +++ b/test/integration/suggest/patch.test.ts @@ -21,7 +21,8 @@ import type { App } from "obsidian"; import type { LinkSuggestion } from "../../../src/suggest/item.ts"; import { installFluentTitles } from "../../../src/suggest/patch.ts"; -const OPTIONS = { property: "fluent" }; +/** The patch reads its settings per call, so a test supplies them the same way the plugin does. */ +const options = () => ({ property: "fluent" }); /** Stands in for the `TFile` on a suggestion. */ const file: any = { basename: "Interiority", path: "Interiority.md" }; @@ -73,8 +74,8 @@ function appWith(suggests: unknown[], frontmatter: unknown = { fluent: true }): * outlived its test would still be wrapping the next one's — and wrapping it *underneath*, so the * inner patch would quietly transform a suggestion the outer one had decided to leave alone. */ -function installFor(t: TestContext, app: App) { - const result = installFluentTitles(app, OPTIONS); +function installFor(t: TestContext, app: App, provider = options) { + const result = installFluentTitles(app, provider); assert.equal(result.installed, true, "expected the fake registry to be patchable"); t.after(() => { @@ -186,6 +187,27 @@ test("a note that is not fluent reaches the composer exactly as it left the popu assert.equal(suggest.received[0]?.item, item); }); +test("the property is read on every completion, not captured at install", (t) => { + // Renaming the property in settings has to take effect on the next completion. The patch is + // installed once and left in place, so the only thing that can carry a rename across is reading + // the settings per call — a wrapper that closed over them would keep answering with the name the + // vault had when the plugin loaded. + const suggest = new LinkSuggest(); + const settings = { property: "fluent" }; + installFor(t, appWith([suggest]), () => settings); + + suggest.context = contextFor("about the "); + suggest.selectSuggestion(fileItem(), {}); + assert.equal((suggest.received[0]?.item as any).alias, "interiority", "the vault's property is fluent"); + + settings.property = "common-noun"; + suggest.context = contextFor("about the "); + const item = fileItem(); + suggest.selectSuggestion(item, {}); + + assert.equal(suggest.received[1]?.item, item, "the note no longer carries the property being looked for"); +}); + test("uninstalling puts the completer back", (t) => { const { suggest, result } = install(t); assert.equal(result.installed, true); @@ -199,6 +221,26 @@ test("uninstalling puts the completer back", (t) => { assert.equal(suggest.received[0]?.item, item, "the wrapper should be gone, not merely inert"); }); +test("installing again after an uninstall patches afresh", (t) => { + // The master toggle removes the patch and puts it back, so the completer is wrapped, unwrapped + // and wrapped again within one session. That is not the same code path as the first install: + // `selectSuggestion` is inherited rather than owned here, exactly as it is in the app, so + // uninstalling *deletes* the wrapper off the subclass instead of restoring a property — and the + // second install has to wrap what the prototype chain is offering by then. + const suggest = new LinkSuggest(); + const app = appWith([suggest]); + + const first = installFluentTitles(app, options); + assert.equal(first.installed, true); + if (first.installed) first.uninstall(); + + installFor(t, app); + suggest.context = contextFor("about the "); + suggest.selectSuggestion(fileItem(), {}); + + assert.equal((suggest.received[0]?.item as any).alias, "interiority", "the second install must adjust too"); +}); + test("a registry that does not look as expected is reported rather than thrown", () => { // Failure is a value because an Obsidian update can cause it, and because a plugin that throws // during onload is one that cannot be uninstalled from inside Obsidian. @@ -212,7 +254,7 @@ test("a registry that does not look as expected is reported rather than thrown", ]; for (const app of registries) { - const result = installFluentTitles(app as App, OPTIONS); + const result = installFluentTitles(app as App, options); assert.equal(result.installed, false, `expected no patch for ${JSON.stringify(app)}`); if (!result.installed) assert.match(result.reason, /\S/); } diff --git a/test/unit/settings/defs.test.ts b/test/unit/settings/defs.test.ts new file mode 100644 index 0000000..199e1d2 --- /dev/null +++ b/test/unit/settings/defs.test.ts @@ -0,0 +1,88 @@ +/** + * What Fluidity does with the file it persisted last time. + * + * `data.json` is the one input to this plugin that nothing validates on its way in: it is written + * by us, but it is read back from disk, it can be hand-edited, and it can have been written by a + * version of the plugin that disagreed about the shape. These assertions are the whole of what + * stops a bad one from producing behavior nobody asked for, so each corrupt shape below is one the + * naive merge would have accepted. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { DEFAULT_SETTINGS, normalizeProperty, normalizeSettings } from "../../../src/settings/defs.ts"; + +test("a first run, with nothing stored, gets the defaults", () => { + assert.deepEqual(normalizeSettings(null), DEFAULT_SETTINGS); + assert.deepEqual(normalizeSettings(undefined), DEFAULT_SETTINGS); + assert.deepEqual(normalizeSettings({}), DEFAULT_SETTINGS); +}); + +test("fluent titles are on until someone turns them off", () => { + // The default matters on its own: it is what a vault gets from installing the plugin and doing + // nothing else, and the feature reference states it. + assert.equal(DEFAULT_SETTINGS.fluentTitles, true); + assert.equal(DEFAULT_SETTINGS.fluentProperty, "fluent"); +}); + +test("stored settings are read back as they were saved", () => { + assert.deepEqual(normalizeSettings({ fluentTitles: false, fluentProperty: "common-noun" }), { + fluentTitles: false, + fluentProperty: "common-noun", + }); +}); + +test("a toggle that is not a boolean falls back rather than being believed", () => { + // `"false"` is the dangerous one: it is truthy, so a merge would turn the feature on for someone + // whose file says it is off. + assert.equal(normalizeSettings({ fluentTitles: "false" }).fluentTitles, true); + assert.equal(normalizeSettings({ fluentTitles: 0 }).fluentTitles, true); + assert.equal(normalizeSettings({ fluentTitles: null }).fluentTitles, true); +}); + +test("a property that is not a string falls back rather than being believed", () => { + // `null` is the dangerous one: a merge keeps it, and reading frontmatter under it looks for a + // property literally named "null". + assert.equal(normalizeSettings({ fluentProperty: null }).fluentProperty, "fluent"); + assert.equal(normalizeSettings({ fluentProperty: 42 }).fluentProperty, "fluent"); + assert.equal(normalizeSettings({ fluentProperty: ["fluent"] }).fluentProperty, "fluent"); +}); + +test("one unusable field does not drag the other back to its default", () => { + // The two settings are independent, and a hand-edit that breaks one should leave the other + // saying what its owner meant. + assert.deepEqual(normalizeSettings({ fluentTitles: false, fluentProperty: 42 }), { + fluentTitles: false, + fluentProperty: "fluent", + }); +}); + +test("the stored record is rebuilt, not extended", () => { + // Anything else in the file — a setting this version does not have, or one a user invented — + // does not survive into the record the plugin runs on. + const normalized = normalizeSettings({ fluentTitles: true, fluentProperty: "fluent", sectionLinks: true }); + + assert.deepEqual(Object.keys(normalized).sort(), ["fluentProperty", "fluentTitles"]); +}); + +test("surrounding whitespace in a property name is dropped", () => { + // Invisible in the settings field and invisible in the property editor, so a stray space would + // read as the feature being broken rather than as a typo. + assert.equal(normalizeProperty(" fluent "), "fluent"); + assert.equal(normalizeProperty("\tcommon-noun\n"), "common-noun"); +}); + +test("an empty property name means the default", () => { + // This is what lets the settings field be cleared: empty shows the placeholder and behaves as + // `fluent`, rather than matching a property with no name. + assert.equal(normalizeProperty(""), "fluent"); + assert.equal(normalizeProperty(" "), "fluent"); +}); + +test("a property name is otherwise the user's own vocabulary", () => { + // Not lowercased, not slugified, not checked against anything: the only wrong answer is one that + // cannot name a property at all. + assert.equal(normalizeProperty("Fluent Noun"), "Fluent Noun"); + assert.equal(normalizeProperty("流暢"), "流暢"); +});