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("流暢"), "流暢"); +});