diff --git a/.gitignore b/.gitignore index 1b18dcb..f1b4140 100644 --- a/.gitignore +++ b/.gitignore @@ -12,8 +12,10 @@ npm-debug.log* /result /result-* -# Obsidian plugin data, present when the repository is symlinked into a vault -# for testing. +# Obsidian plugin data. `make link` links the plugin's files individually, so a +# vault writes its settings beside them rather than here; this covers a vault +# whose whole plugin folder is a symlink to the checkout, which `make unlink` +# copies across. /data.json # The local development runbook: session state and working notes, not user @@ -25,3 +27,6 @@ npm-debug.log* *.swp /.idea/ /.vscode/ + +# The local-only plan directory +tmp/ diff --git a/Makefile b/Makefile index 207d4a9..9cd884b 100644 --- a/Makefile +++ b/Makefile @@ -19,9 +19,53 @@ help: ## Show this help # -- Building ------------------------------------------------------------------------------------- +# npm installs only the esbuild binary for the platform it runs on, and one checkout may be shared +# between platforms over a single node_modules — a container that builds and a host that runs +# Obsidian, for example. An install on either would otherwise leave the other with "You installed +# esbuild for another platform", so every binary that shares this checkout is put in place +# afterwards, at whatever version the lockfile resolved esbuild to. +# +# They cannot be declared in package.json: npm skips a dependency whose `os` does not match the +# machine installing it, and prunes it from node_modules on the next install. +# +# They are fetched with `npm pack` and unpacked because `npm install` refuses a foreign platform +# outright, and the one flag that overrides that, `--force`, also drops the engine and +# dependency-conflict checks `npm ci` has just applied and lets a fresh resolution rearrange the +# tree it laid down. `npm pack` only downloads; extracting the tarball into place touches nothing +# else in node_modules. +# +# Every esbuild in the tree is repaired, not just the top-level one: tsx carries its own copy at a +# different version, and a binary is only ever found beside the host that asks for it. Leaving a +# nested host to fall back on the top-level binary is how `Host version X does not match binary +# version Y` happens, which stops the test runner dead while the build itself is fine. +ESBUILD_PLATFORMS := @esbuild/darwin-arm64 @esbuild/linux-arm64 + +# Prints the `version` field of the package.json given as its argument. +pkg-version = sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' + .PHONY: deps -deps: ## Install npm dependencies exactly as locked +deps: ## Install npm dependencies exactly as locked, for every platform sharing this checkout $(RUN) npm ci + @if [ -n "$(CI)" ]; then \ + echo "CI: skipping the cross-platform esbuild repair — a runner shares its node_modules with nothing."; \ + exit 0; \ + fi; \ + tmp=$$(mktemp -d); \ + trap 'rm -rf "$$tmp"' EXIT; \ + for host in $$(find node_modules -path '*/esbuild/package.json' -not -path '*/@esbuild/*'); do \ + nm=$$(dirname "$$(dirname "$$host")"); \ + v=$$($(pkg-version) "$$host" | head -1); \ + for p in $(ESBUILD_PLATFORMS); do \ + have=$$($(pkg-version) "$$nm/$$p/package.json" 2>/dev/null | head -1); \ + if [ "$$have" != "$$v" ]; then \ + tarball=$$($(RUN) npm pack --silent --pack-destination "$$tmp" "$$p@$$v") || exit 1; \ + rm -rf "$$nm/$$p"; \ + mkdir -p "$$nm/$$p"; \ + tar -xzf "$$tmp/$$tarball" -C "$$nm/$$p" --strip-components=1 || exit 1; \ + echo "Unpacked $$p@$$v into $$nm."; \ + fi; \ + done; \ + done .PHONY: build build: typecheck ## Produce a release main.js @@ -61,113 +105,192 @@ check: format-check typecheck lint test ## Everything CI checks # -- Installing ----------------------------------------------------------------------------------- -# `install` and `link` are two ways into the same vault folder, so they share these guards. `$@` -# expands where the block is used, which is what lets one copy name the target the user actually -# ran. Both checks happen before anything expensive, so a typo in DEV_VAULT_PATH costs a second -# rather than a full build. +# `install`, `link` and `unlink` are three ways into the same vault folder, so they share these +# guards. `$@` expands where the block is used, which is what lets one copy name the target the +# user actually ran. Both checks happen before anything expensive, so a typo in DEV_VAULT_PATH +# costs a second rather than a full build. define vault-guard @if [ -z "$(DEV_VAULT_PATH)" ]; then \ - echo "make $@: DEV_VAULT_PATH is not set." >&2; \ - echo " DEV_VAULT_PATH=~/vaults/dev make $@" >&2; \ - exit 1; \ + echo "make $@: DEV_VAULT_PATH is not set." >&2; \ + echo " DEV_VAULT_PATH=~/vaults/dev make $@" >&2; \ + exit 1; \ fi @if [ ! -d "$(DEV_VAULT_PATH)/.obsidian" ]; then \ - echo "make $@: '$(DEV_VAULT_PATH)' is not an Obsidian vault (no .obsidian directory)." >&2; \ - exit 1; \ + echo "make $@: '$(DEV_VAULT_PATH)' is not an Obsidian vault (no .obsidian directory)." >&2; \ + exit 1; \ fi endef -# main.js, manifest.json and styles.css are the three files Obsidian installs, and the folder they -# live in must be named after manifest.json's `id`. Read the id from the manifest rather than -# hardcoding it, so a rename cannot leave this target installing into a stale directory. +# The three files Obsidian installs, in the folder it loads a plugin from. +PLUGIN_FILES := main.js manifest.json styles.css + +# That folder must be named after manifest.json's `id`, which is read out of the manifest rather +# than hardcoded, so a rename cannot leave these targets working on a stale directory. It is read +# with sed rather than node because a machine that only runs Obsidian needs no toolchain to manage +# its own vault, and requiring one is what turns a broken install into a hand-written `rm`. +plugin-id = sed -n 's/.*"id"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' manifest.json | head -1 + +# Where Obsidian loads this plugin from inside `$DEV_VAULT_PATH`, named once because three targets +# act on it: a drift between them would have one working on a directory another had ruled out. +vault-dest = id=$$($(plugin-id)); dest="$(DEV_VAULT_PATH)/.obsidian/plugins/$$id" + +# What is at the destination, in one word: +# +# links a real directory holding symlinks into a checkout — what `make link` builds +# whole-link the directory is itself one symlink to a checkout +# copied a real directory of real files — an install, possibly with its data.json +# other something else, which nothing here will touch +# absent nothing +# +# `whole-link` is not a shape any target here produces, but a vault may still carry one and it is +# the shape worth naming, because it is the one that makes `rm` dangerous. See `link`. +dest-shape = if [ -L "$$dest" ]; then echo whole-link; elif [ -L "$$dest/main.js" ]; then echo links; elif [ -d "$$dest" ]; then echo copied; elif [ -e "$$dest" ]; then echo other; else echo absent; fi + +# Which checkout a linked destination follows, for either linked shape. Empty when it resolves to +# nothing. Shape alone is not enough to act on: a vault may be linked to a *different* checkout of +# this repository, and every target here would otherwise treat that as its own. +dest-target = if [ -L "$$dest" ]; then cd "$$dest" 2>/dev/null && pwd -P; elif [ -L "$$dest/main.js" ]; then t=$$(readlink "$$dest/main.js"); [ -n "$$t" ] && cd "$$dest" 2>/dev/null && cd "$$(dirname "$$t")" 2>/dev/null && pwd -P; fi + # The destination is checked before `build` rather than alongside it; that is what the recursive # `$(MAKE)` buys, since prerequisite order is not guaranteed under `-j`. .PHONY: install install: ## Build and install into the vault at $DEV_VAULT_PATH $(vault-guard) @$(MAKE) --no-print-directory build - @id=$$($(RUN) node -p "require('./manifest.json').id"); \ - dest="$(DEV_VAULT_PATH)/.obsidian/plugins/$$id"; \ - if [ -L "$$dest" ] && [ "$$(cd "$$dest" 2>/dev/null && pwd -P)" = "$$(pwd -P)" ]; then \ - echo "$$dest is a symlink to this checkout, so the build it already sees is the one just made."; \ - echo "Reload Obsidian, or disable and re-enable the plugin, to pick it up."; \ - exit 0; \ - fi; \ - mkdir -p "$$dest"; \ - cp main.js manifest.json styles.css "$$dest/" || exit 1; \ - echo "Installed $$id into $$dest."; \ - echo "Reload Obsidian, or disable and re-enable the plugin, to pick the build up." - -# `link` is `install` without the copy: Obsidian follows a symlinked plugin folder, so pointing one -# at the checkout makes `make dev`'s rebuilt main.js live in the vault with no second step. It -# deliberately does not build — you link once and then leave `make dev` running — so a fresh -# checkout has no main.js yet, hence the reminder rather than a silent broken plugin. An existing -# destination is never removed here: a real directory is somebody's `make install` output (or, on a -# vault that has ever had the published plugin, their settings), and deleting either to save a -# `rm` is not a trade this target gets to make. + @$(vault-dest); \ + case $$($(dest-shape)) in \ + links|whole-link) \ + target=$$($(dest-target)); \ + if [ "$$target" = "$$(pwd -P)" ]; then \ + echo "$$dest links to this checkout, so the build it already sees is the one just made."; \ + echo "Reload Obsidian, or disable and re-enable the plugin, to pick it up."; \ + else \ + echo "make install: '$$dest' links to '$${target:-a checkout that cannot be resolved}', not this one." >&2; \ + echo " Obsidian would go on loading that one, so this build has not been installed." >&2; \ + echo " 'make unlink' replaces the links with this checkout's build." >&2; \ + exit 1; \ + fi; \ + ;; \ + other) \ + echo "make install: '$$dest' exists and is not a plugin directory." >&2; \ + exit 1; \ + ;; \ + *) \ + mkdir -p "$$dest"; \ + for f in $(PLUGIN_FILES); do rm -f "$$dest/$$f"; done; \ + cp $(PLUGIN_FILES) "$$dest/" || exit 1; \ + echo "Installed $$id into $$dest."; \ + echo "Reload Obsidian, or disable and re-enable the plugin, to pick the build up."; \ + ;; \ + esac + +# `link` is `install` without the copy: the plugin folder is real, and the files inside it are +# symlinks into this checkout, so `make dev`'s rebuilt main.js is live in the vault with no second +# step. +# +# Linking the files rather than the folder is what makes the arrangement safe to remove. `rm` +# deletes a symlink instead of following it, so every way of clearing the plugin folder out costs +# three links this target rebuilds in a second, and none of them can reach the checkout. Linking +# the folder gives that same `rm` a way through: a trailing slash — which shell completion appends +# 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. .PHONY: link -link: ## Symlink this checkout into the vault at $DEV_VAULT_PATH (pairs with make dev) +link: ## Symlink this checkout's files into the vault at $DEV_VAULT_PATH (pairs with make dev) $(vault-guard) - @id=$$($(RUN) node -p "require('./manifest.json').id"); \ - dest="$(DEV_VAULT_PATH)/.obsidian/plugins/$$id"; \ + @$(vault-dest); \ here=$$(pwd -P); \ - if [ -L "$$dest" ]; then \ - target=$$(cd "$$dest" 2>/dev/null && pwd -P); \ - if [ "$$target" = "$$here" ]; then \ - echo "Already linked: $$dest -> $$here"; \ - elif [ -z "$$target" ]; then \ - echo "make link: '$$dest' is a broken symlink." >&2; \ - echo " Remove it and re-run: rm '$$dest'" >&2; \ - exit 1; \ - else \ - echo "make link: '$$dest' already links to '$$target', not this checkout." >&2; \ - echo " Remove it and re-run: rm '$$dest'" >&2; \ - exit 1; \ - fi; \ - elif [ -e "$$dest" ]; then \ - echo "make link: '$$dest' exists and is a real directory, not a symlink." >&2; \ - echo " It 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; \ - else \ - mkdir -p "$$(dirname "$$dest")"; \ - ln -s "$$here" "$$dest"; \ - echo "Linked $$dest -> $$here."; \ + 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; \ + ;; \ + 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; \ + ;; \ + other) \ + echo "make link: '$$dest' exists and is not a plugin directory." >&2; \ + exit 1; \ + ;; \ + esac; \ + target=$$($(dest-target)); \ + if [ -n "$$target" ] && [ "$$target" != "$$here" ]; then \ + echo "make link: '$$dest' links to '$$target', not this checkout." >&2; \ + echo " Its contents are symlinks, so removing it reaches no checkout: rm -r '$$dest'" >&2; \ + exit 1; \ fi; \ + mkdir -p "$$dest"; \ + for f in $(PLUGIN_FILES); do \ + ln -sfn "$$here/$$f" "$$dest/$$f" || exit 1; \ + done; \ + echo "Linked $$dest -> $$here ($(PLUGIN_FILES))."; \ if [ ! -f main.js ]; then \ - echo "No main.js yet: run 'make dev' (or 'make build') before enabling the plugin."; \ + echo "No main.js yet: run 'make dev' (or 'make build') before enabling the plugin."; \ fi -# `unlink` is the way back from `link` to an ordinary install: drop the symlink and leave the three -# files Obsidian ships in its place. The build runs *before* the symlink goes, so a failing build -# leaves the vault with the plugin it already had rather than an empty folder. `data.json` comes -# along if the checkout has one, because that is where a linked plugin has been writing its -# settings, and silently resetting them on the way back would be a poor trade for a `cp`. +# `unlink` is the way back from `link` to an ordinary install: replace the links with the files +# they point at. +# +# Its whole job is to get a vault off a checkout, so a build it cannot run is not allowed to stop +# it. The machine holding the vault may have no working toolchain at all — a shared node_modules +# carries one platform's binaries at a time — and failing there would leave a hand-written `rm` as +# the only way out, which is the thing this target exists to spare you. A failed build with a +# main.js already present installs that one and says so; only a destination with nothing at all to +# copy is an error. +# +# `data.json` is carried across from the checkout when the whole folder was a link, because that is +# where a plugin linked that way has been writing its settings. Under `link` it is already a real +# file in the vault, and is left alone. .PHONY: unlink -unlink: ## Replace the $DEV_VAULT_PATH symlink with a copied build +unlink: ## Replace the $DEV_VAULT_PATH links with a copied build $(vault-guard) - @id=$$($(RUN) node -p "require('./manifest.json').id"); \ - dest="$(DEV_VAULT_PATH)/.obsidian/plugins/$$id"; \ - if [ ! -L "$$dest" ]; then \ - if [ -d "$$dest" ]; then \ - echo "make unlink: '$$dest' is a real directory, not a symlink." >&2; \ - echo " It is already a copied install; 'make install' refreshes it in place." >&2; \ - else \ - echo "make unlink: nothing is linked at '$$dest'." >&2; \ - echo " 'make install' puts a copied build there." >&2; \ - fi; \ - exit 1; \ + @$(vault-dest); \ + shape=$$($(dest-shape)); \ + case $$shape in \ + links|whole-link) ;; \ + copied) \ + echo "make unlink: '$$dest' is already a copied install, not a link." >&2; \ + echo " 'make install' refreshes it in place." >&2; \ + exit 1; \ + ;; \ + *) \ + echo "make unlink: nothing is linked at '$$dest'." >&2; \ + echo " 'make install' puts a copied build there." >&2; \ + exit 1; \ + ;; \ + esac; \ + was=$$($(dest-target)); \ + if [ -n "$$was" ] && [ "$$was" != "$$(pwd -P)" ]; then \ + echo "make unlink: '$$dest' links to '$$was'; this checkout's build is what replaces it." >&2; \ fi; \ - was=$$(cd "$$dest" 2>/dev/null && pwd -P); \ - $(MAKE) --no-print-directory build || exit 1; \ - rm "$$dest"; \ - mkdir -p "$$dest"; \ - cp main.js manifest.json styles.css "$$dest/" || exit 1; \ - if [ -f data.json ]; then \ - cp data.json "$$dest/" || exit 1; \ - echo "Copied data.json across, so the plugin keeps the settings it had while linked."; \ + if ! $(MAKE) --no-print-directory build; then \ + if [ -f main.js ]; then \ + echo "make unlink: the build failed, so the main.js already in this checkout is the one installed." >&2; \ + else \ + echo "make unlink: the build failed, and there is no main.js to install instead." >&2; \ + echo " '$$dest' is untouched, and still links to $${was:-somewhere unresolvable}." >&2; \ + exit 1; \ + fi; \ + fi; \ + if [ "$$shape" = whole-link ]; then \ + rm "$$dest"; \ + mkdir -p "$$dest"; \ + else \ + for f in $(PLUGIN_FILES); do rm -f "$$dest/$$f"; done; \ + fi; \ + cp $(PLUGIN_FILES) "$$dest/" || exit 1; \ + if [ "$$shape" = whole-link ] && [ -n "$$was" ] && [ -f "$$was/data.json" ]; then \ + cp "$$was/data.json" "$$dest/" || exit 1; \ + echo "Copied data.json across from $$was, so the plugin keeps the settings it had while linked."; \ fi; \ - echo "Unlinked $$id: $$dest is a copy of the build, and no longer a symlink to $$was."; \ + echo "Unlinked $$id: $$dest holds a copy of the build, and no longer links to $${was:-anything}."; \ echo "Reload Obsidian, or disable and re-enable the plugin, to pick it up." # -- Utility -------------------------------------------------------------------------------------- diff --git a/README.md b/README.md index 6a0ee4b..8958d18 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ Obsidian's link completer is not exposed, so Fluidity achieves what it does by b and **patching it**. It finds the built-in suggester at runtime, wraps the method that turns your choice into text, and then adjusts the choice before handing it to Obsidian's code to insert. The insertion itself is never reimplemented, which is why the result respects your link-format settings -and why undo puts the note back in a single step. +and why undoing a link costs no more than undoing any other completion. The cost of doing it this way is simply that an Obsidian update can trivially break the plugin. Fluidity is designed to fail gracefully when this happens, not installing the patch and reporting diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index e24b6a7..6e32776 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -21,7 +21,7 @@ project's toolchain. CI runs inside the same shell, executing the same `make` ta git clone https://github.com/iamrecursion/fluidity.git cd fluidity nix develop # or: make shell -make deps # npm ci +make deps # npm ci, plus the esbuild binary for every platform sharing the checkout make build # type-check and bundle main.js make check # exactly what CI runs ``` @@ -31,17 +31,17 @@ 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 into `$DEV_VAULT_PATH` instead of copying | -| `make unlink` | swap that symlink 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 | +| `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 @@ -68,9 +68,10 @@ looks for. The target refuses a path with no `.obsidian` directory in it, and ch building rather than after. Obsidian does not notice the new files on its own, so you will need to reload the app (or toggle the plugin off and back on). -For rapid development, `make link` symlinks the repository into `/.obsidian/plugins/` instead -of copying into it. Obsidian follows the symlink, so a rebuild is live in the vault with no second -step, which pairs well with leaving `make dev` running. +For rapid development, `make link` fills `/.obsidian/plugins//` with symlinks to this +checkout's `main.js`, `manifest.json` and `styles.css` instead of copying them. Obsidian follows +each one, so a rebuild is live in the vault with no second step, which pairs well with leaving +`make dev` running. ```sh export DEV_VAULT_PATH=~/vaults/dev @@ -78,24 +79,34 @@ 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, 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 real folder there, `make link` tells you to -delete it yourself, because that folder may hold your `data.json`. Once linked, the settings -Obsidian writes land in the checkout itself, which the `.gitignore` already accounts for. Reloading -is still on you as Obsidian does not watch the file for changes. - -`make unlink` is the way back. It builds, removes the symlink, and copies the same three files in -its place, leaving the vault with an ordinary install: +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 +Obsidian writes land in the vault folder beside the links. Reloading is still on you, as Obsidian +does not watch the files for changes. + +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 +that property: `rm -rf /`, carrying the trailing slash that shell completion appends for +you, deletes the contents of the checkout it points at, silently and with nothing reported. + +`make unlink` is the way back. It replaces the links with the files they point at, leaving the vault +with an ordinary install: ```sh export DEV_VAULT_PATH=~/vaults/dev make unlink ``` -The build happens **before** the symlink goes, so a build that fails leaves the vault with the -plugin it already had rather than an empty folder. The checkout's `data.json` is copied across if -there is one, since that is where the linked plugin has been keeping its settings. +Its job is to get a vault off a checkout, so a build it cannot run does not stop it: with a +`main.js` already in the checkout it installs that one and says so, and only a destination with +nothing to copy at all is an error. That matters on a machine which only runs Obsidian, where the +toolchain may not work and where failing would leave a hand-written `rm` as the only way out. + +A folder that is itself a symlink to a whole checkout is converted too, and that checkout's +`data.json` is copied across, since that is where a plugin linked that way keeps its settings. ### What to Check by Hand @@ -106,8 +117,11 @@ Any change to what gets inserted should be exercised against at least this much: 2. A **multi-word** fluent title, and one of its **aliases**. 3. A note with no `fluent` property, and one with `fluent: false`, which should both be completely untouched. -4. **Undo**, which must put the note back in one step. If it takes two, the plugin is rewriting text - after insertion somewhere, and that is a bug regardless of what the undo produces. +4. **Undo**, which must cost exactly as many steps as it does with the plugin disabled. Accepting + any completion takes two in stock Obsidian, one for the completion and one for the typing, so + count both ways rather than expecting one. A fluent insertion costing more than a control does + means the plugin is rewriting text after insertion, and that is a bug regardless of what the undo + produces. 5. Selection by **mouse click**, by **Enter**, and by **Tab**. 6. `#`, `^` and `|` completions, which must behave exactly as they do without the plugin. 7. **Use `[[Wikilinks]]` turned off**, where the same choice must produce a well-formed Markdown @@ -183,7 +197,8 @@ pull request that breaks one of them will be sent back: - **Read what you need before delegating** as the original clears the state you want to read in its first statement. - **Never reimplement the insertion.** Adjust the suggestion and hand it back. That is what makes - the result honor link-format settings, and what makes undo a single step. + the result honor link-format settings, and what keeps the insertion one editor transaction, so + undo costs no more than it does without the plugin. - **Pass through what you do not handle**, untouched and by identity. Some suggestion types write to files when selected, and intercepting one of those is how a plugin corrupts a note. - **Fail quietly** if the patch cannot be installed, log one line naming the plugin, disable the diff --git a/docs/architecture.md b/docs/architecture.md index dccc2ce..652201c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -137,6 +137,11 @@ how it does so is a deliberate response to something that will otherwise go wron `suggests: EditorSuggest[]`. Dispatch is **first non-null `onTrigger` wins, in array order**, which is how one plugin's suggester displaces another's. +Because that singleton is built with the workspace, the registry is already populated by the time a +plugin's `onload` runs, which is why the patch is not deferred to `onLayoutReady`. Enabling a plugin +by hand happens long after startup, so a patch that needed the later hook would work every time it +was switched on and be dead on every cold start. + The built-in link suggester is `suggests[0]` at startup, but it **must be located by capability**: ```ts @@ -177,8 +182,12 @@ control is handed to the original: const ctx = this.context; // before old(), never after ``` -This is the kind of thing that works perfectly in every test written against a fake and fails on the -first real keystroke. +This is the kind of thing that works perfectly against a fake whose `selectSuggestion` leaves the +context alone, and fails on the first real keystroke. The fake in +`test/integration/suggest/patch.test.ts` clears it, exactly as `close()` does: a wrapper that reads +the context anywhere but off the instance being called — when the patch is installed, say — then +sees the cleared value, stops adjusting anything. That file pins the rest of the rules in this +section the same way, each against a registry shaped like the real one. ### Only Some Suggestion Types May be Touched @@ -208,8 +217,8 @@ silently write the wrong link — which is worth stating plainly, because "use t otherwise the obviously correct advice. Going through the built-in composer also inherits correct handling of both **Use [[Wikilinks]]** and -**New link format** for free, and keeps the insertion a single editor transaction, which is what -makes undo one step. +**New link format** for free, and keeps the insertion a single editor transaction, so undoing it +costs exactly what undoing any completion costs. ### A Fluent Note With no Alias Becomes an Alias Item diff --git a/docs/features.md b/docs/features.md index 97e85f9..af7e6c6 100644 --- a/docs/features.md +++ b/docs/features.md @@ -19,7 +19,7 @@ single sentence answers most questions about scope: the same in any other Markdown editor. - It does not run on typing, on paste, or on save. - It never rewrites text after Obsidian has inserted it. The link Obsidian writes is already the - right one, which is why **undo is a single step**. + right one, so **undoing it costs exactly what undoing any completion costs**. ## Fluent Note Titles @@ -103,6 +103,10 @@ makes them readable in a list. ### Other Completions are Untouched +**Embeds are left alone.** `![[Interiority]]` renders the note rather than reading as prose, and +what follows the pipe in one is a display argument rather than text. An embed is inserted exactly as +Obsidian would insert it, whether or not the note is fluent. + Typing `#` for a heading, `^` for a block reference, or `|` for an alias inside a link behaves exactly as it does without the plugin. So does `Shift+Enter`, and so does every suggestion type Fluidity does not explicitly handle. In particular, block-reference completions are never @@ -206,6 +210,10 @@ 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 | @@ -222,3 +230,6 @@ the first thing to check when nothing seems to be happening: Fluidity works by p 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. + +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. diff --git a/src/fluent/display.ts b/src/fluent/display.ts new file mode 100644 index 0000000..dd07306 --- /dev/null +++ b/src/fluent/display.ts @@ -0,0 +1,30 @@ +/** + * The casing rule: what a fluent note's link should actually say. + * + * Every casing decision in the plugin goes through this one function, which is what makes the rule + * replaceable. The roadmap's per-note override language changes this body and nothing else. + * + * It is separate from `suggest/transform` because that module decides *whether* a suggestion is + * ours to touch and this one decides *what the text becomes*; the first is about Obsidian's + * internals and the second is about English. + */ + +/** + * The display text to insert for a fluent note, given the text Obsidian would have inserted. + * + * Whether the note is fluent at all is settled before this is called — that question belongs to + * `fluent/frontmatter`, and asking it again here would mean two modules answering it. + * + * The rule is blunt on purpose: it lowercases the whole display text, so + * `Object Oriented Programming` comes out right and `History of France` does not. That cost is + * accepted for the first version, and documented where users will meet it — the point of v1 is to + * establish that patching the completer is reliable, and a casing rule with no configuration + * surface keeps that question clean. + * + * Lowercasing is locale-aware, so non-ASCII scripts behave as the reader's locale expects rather + * than as ASCII would have it. + */ +export function fluentDisplay(displayText: string, atSentenceStart: boolean): string { + if (atSentenceStart) return displayText; + return displayText.toLocaleLowerCase(); +} diff --git a/src/fluent/frontmatter.ts b/src/fluent/frontmatter.ts new file mode 100644 index 0000000..f26e33a --- /dev/null +++ b/src/fluent/frontmatter.ts @@ -0,0 +1,26 @@ +/** + * Reading fluency out of a note's frontmatter. + * + * The property is read from a plain object rather than from a file, which keeps this module free + * of Obsidian and lets the caller decide where the frontmatter came from. In the plugin that is + * always the metadata cache, so there is no file read between a keypress and an insertion. + * + * It is separate from `fluent/display` because this one is about what a note says about itself and + * that one is about the text being written; the second feature reads the same frontmatter for a + * different purpose, and will read it through here. + */ + +/** + * Is this note marked fluent — does its title name a common noun rather than a proper one? + * + * The value must be a real boolean. `fluent: "true"` is a string and does not mark the note, which + * is deliberate: Obsidian's property editor writes a real boolean for a checkbox property, so a + * string is a typo, and silently honoring one would mean a typo changing how links are written. + * + * `frontmatter` is typed loosely because that is what it is — a parsed YAML mapping, which may be + * absent, may be any shape, and is not ours to trust. + */ +export function isFluent(frontmatter: unknown, property: string): boolean { + if (typeof frontmatter !== "object" || frontmatter === null) return false; + return (frontmatter as Record)[property] === true; +} diff --git a/src/main.ts b/src/main.ts index 22aff54..6534707 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,15 +1,31 @@ /** * Fluidity's entry point. * - * This module owns lifecycle and nothing else: loading settings, installing the completer patch, - * and registering the settings tab, delegating every decision to the modules beneath it. Keeping - * it thin is what lets the rest of the plugin stay testable outside Obsidian. + * 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. * - * It is a placeholder for now — a plugin Obsidian will load and unload cleanly, so that the - * scaffold has something real to build, check and install. The lifecycle arrives with the - * fluent-titles feature. + * 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. */ import { Plugin } from "obsidian"; -export default class FluidityPlugin extends Plugin {} +import { installFluentTitles } from "./suggest/patch"; + +/** The frontmatter property that marks a note fluent. */ +const FLUENT_PROPERTY = "fluent"; + +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; + } + + // 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`); + } +} diff --git a/src/prose/sentence.ts b/src/prose/sentence.ts new file mode 100644 index 0000000..bf00be1 --- /dev/null +++ b/src/prose/sentence.ts @@ -0,0 +1,126 @@ +/** + * Where a sentence begins, judged from the Markdown preceding a link. + * + * This module is the reason the casing rule is safe to apply at all: lowercasing a display text is + * only right when the link sits mid-sentence, and the only evidence for that is the text already + * on the line. It takes a string and answers a question about it, so it imports nothing and is + * exercised entirely from `test/unit`. + * + * It is separate from `fluent/display` because the two answer different questions — this one is + * about English and Markdown, that one is about a note's title — and because the second feature + * will need this judgement without needing the casing rule. + */ + +/** + * Markers that may sit between the end of a sentence's preceding text and the link, without + * moving where the sentence starts. + * + * Emphasis opened immediately before a link is part of the link's presentation, not part of the + * prose, so `**[[Interiority]]** is …` starts a sentence. Opening quotes and brackets are the same + * case: `("[[Interiority]] …` does too. Closing marks are deliberately absent — text before a + * closing bracket is prose that has already begun. + */ +const TRANSPARENT_BEFORE_LINK = new Set([ + // Emphasis: bold, italic, highlight and strikethrough, in every length they come in. + "*", + "_", + "=", + "~", + // Opening quotes and brackets, straight and curly, including the ones other languages open with. + "\"", + "'", + "“", + "‘", + "„", + "«", + "‹", + "(", + "[", + "{", +]); + +/** + * A blockquote or callout opener, which the rest of a line's structure may sit inside. + * + * Stripping it before anything else is what lets one set of patterns describe a heading, a bullet + * or an ordered marker whether or not it is quoted. `> - ` is a bullet in a quote, and the bullet + * is the part that says a sentence begins; `>` on its own says so too, having nothing after it. + */ +const QUOTE_PREFIX = /^\s*(?:>\s*)+(?:\[![^\]]*\][+-]?\s*)?/; + +/** A line that is only Markdown structure: whatever follows it begins the line's prose. */ +const STRUCTURE_ONLY = [ + // A heading marker, `#` through `######`. + /^\s*#{1,6}$/, + // A bullet, on its own or carrying a checkbox. + /^\s*[-*+](?:\s+\[.\])?$/, + // An ordered-list marker. + /^\s*\d+[.)]$/, +]; + +/** + * Sentence-ending punctuation, which must be followed by whitespace to count. + * + * `:` and `;` are here as a judgement call rather than a grammatical rule: a colon is how most + * people write a lead-in, and what follows one reads as a new clause. The feature reference says + * so plainly, and records it as the behavior most likely to become a setting. + * + * Closing quotes and brackets may sit between the punctuation and the space, because a sentence + * that ends inside quotation marks or parentheses has still ended: `He said "It ends." ` and + * `(see above.) ` both close one. + */ +const TERMINATED = /[.!?:;]["'”’»›)\]}]*\s+$/; + +/** + * The CJK terminators, which do not take a following space in ordinary use. + * + * Their width carries the break that a space carries in Latin script, so requiring whitespace + * after them would mean never recognising a sentence boundary in Japanese or Chinese prose. + */ +const TERMINATED_CJK = /[。！？]\s*$/; + +/** + * A table cell, which begins after the `|` that opens it. + * + * This is matched against the end of the text rather than the whole of it, because the cells + * before it on the row are prose of their own: `| a | [[…]]` opens a cell just as `| [[…]]` does. + * A `|` in ordinary prose reads as a cell opener here too, which keeps a capital that could have + * been lowered — the direction this module fails in everywhere else. + */ +const CELL_OPENER = /\|\s*$/; + +/** + * Does a sentence begin where this text ends? + * + * `before` is the text on the line up to the `[[` that opened the completion — see + * `suggest/transform`, which reads it off the completer's own context rather than inferring it. + * + * Where this is wrong, it is wrong in the direction of answering `true`: the caller then leaves a + * capital alone that might have been lowered, which is visible and fixable, rather than lowering + * one that should have stayed. That is what makes the punctuation cases below safe to keep simple. + * A `.` is treated as a boundary even when it ends a decimal, an abbreviation, a URL or a span of + * inline code; each of those reads as a sentence start here and so keeps its capital. + */ +export function startsSentence(before: string): boolean { + const text = withoutTransparentMarkers(before); + + // Nothing but whitespace: the link opens the line, and a line opens a sentence. This is the one + // place the judgement is made within a single line rather than across the paragraph above it. + if (text.trim() === "") return true; + + // A quote or callout opener is structure in its own right, and what follows it on the line is + // judged exactly as it would be unquoted — so `>`, `> ## `, `> - ` and `> [!note] - ` all open a + // sentence, and a bullet inside a callout is not treated as running prose. + const structure = text.trimEnd().replace(QUOTE_PREFIX, ""); + if (structure === "") return true; + if (STRUCTURE_ONLY.some((pattern) => pattern.test(structure))) return true; + + return TERMINATED.test(text) || TERMINATED_CJK.test(text) || CELL_OPENER.test(text); +} + +/** Drop the run of emphasis, quote and bracket characters sitting directly against the link. */ +function withoutTransparentMarkers(before: string): string { + let end = before.length; + while (end > 0 && TRANSPARENT_BEFORE_LINK.has(before[end - 1])) end -= 1; + return before.slice(0, end); +} diff --git a/src/suggest/item.ts b/src/suggest/item.ts new file mode 100644 index 0000000..fbd15be --- /dev/null +++ b/src/suggest/item.ts @@ -0,0 +1,82 @@ +/** + * The suggestion items the link completer hands to `selectSuggestion`, and the two Fluidity will + * touch. + * + * None of these shapes are published by Obsidian, so they are declared here rather than imported, + * and this file is the single written statement of what the plugin believes they look like. The + * architecture doc records why each belief is held; if one turns out to be wrong, this is the file + * that was wrong. + * + * It is separate from `suggest/transform` so that the shapes can be described once and asserted + * against without pulling in the decision that reads them. + */ + +import type { TFile } from "obsidian"; + +/** A completion for a note itself: its title, with no alias involved. */ +export interface FileSuggestion { + type: "file"; + file: TFile; + path: string; +} + +/** A completion for one of a note's aliases, carrying the text that will follow the `|`. */ +export interface AliasSuggestion { + type: "alias"; + alias: string; + file: TFile; + path: string; +} + +/** + * Everything else the completer can produce — headings, block references, plain link text, and the + * `{ type: "none" }` that `Shift+Enter` passes directly. + */ +export interface UnhandledSuggestion { + type: string; +} + +/** What `selectSuggestion` is called with. */ +export type LinkSuggestion = FileSuggestion | AliasSuggestion | UnhandledSuggestion; + +/** + * Is this a completion for a note's own title? + * + * The checks on the note are not defensive padding. These objects come from code this plugin does + * not own, across an interface nobody has promised to keep, and a guard that only reads `type` + * would hand a malformed item to the transform and turn a missing field into a crash + * mid-keystroke. + */ +export function isFileSuggestion(item: LinkSuggestion): item is FileSuggestion { + return item.type === "file" && carriesBasename(item); +} + +/** + * Is this a completion for one of a note's aliases? + * + * Every other suggestion type is passed through untouched and by identity. That is not tidiness: a + * `block` item writes a block id into the *target* file when it is selected, so a plugin that + * intercepts one and returns something slightly different is a plugin that corrupts notes. + */ +export function isAliasSuggestion(item: LinkSuggestion): item is AliasSuggestion { + return item.type === "alias" && carriesFile(item) && typeof (item as AliasSuggestion).alias === "string"; +} + +/** Does the item carry the note it refers to, as both handled types are expected to? */ +function carriesFile(item: LinkSuggestion): boolean { + const file = (item as { file?: unknown; }).file; + return typeof file === "object" && file !== null; +} + +/** + * Does the item carry a note with the title a file completion's display text comes from? + * + * This is the whole check for a file item, and subsumes `carriesFile`: nothing without a note has + * a `basename` on it. The title is checked rather than assumed because the transform reads + * `file.basename` and lowercases it, so an item whose note lacks one throws on the keystroke — + * caught, but logged, and for a completion that was never ours to touch. + */ +function carriesBasename(item: LinkSuggestion): boolean { + const file = (item as { file?: { basename?: unknown; }; }).file; + return typeof file?.basename === "string"; +} diff --git a/src/suggest/patch.ts b/src/suggest/patch.ts new file mode 100644 index 0000000..7d519bf --- /dev/null +++ b/src/suggest/patch.ts @@ -0,0 +1,130 @@ +/** + * 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 + * 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. + * + * Everything in here is a response to something that otherwise goes wrong, and the architecture + * doc records each one. Nothing in this file throws: a plugin that breaks Obsidian's startup is a + * plugin nobody can uninstall from inside Obsidian. + */ + +import { around } from "monkey-around"; +import type { App, EditorSuggestContext } from "obsidian"; + +import type { LinkSuggestion } from "./item"; +import { type TransformOptions, transformSuggestion } from "./transform"; + +/** + * The built-in link suggester, as much of it as Fluidity depends on. + * + * `context` carries the cursor position, and therefore the words before the link, and therefore + * whether this is a sentence start. + */ +interface LinkSuggest { + context: EditorSuggestContext | null; + constructor: { prototype: LinkSuggestPrototype; }; +} + +type SelectSuggestion = (this: LinkSuggest, item: LinkSuggestion, evt: MouseEvent | KeyboardEvent) => void; + +/** + * The prototype the patch is installed on. + * + * The index signature is `any` because that is what `around` requires of what it wraps: it types + * the object it is handed as `Record`, and an index signature of `unknown` narrows + * every wrapper factory to one that cannot be handed a real method. + */ +interface LinkSuggestPrototype extends Record { + selectSuggestion: SelectSuggestion; +} + +/** + * Whether the patch went on, and how to take it off again. + * + * A failure is a value rather than an exception because it is an expected outcome — an Obsidian + * update is entirely capable of moving what this attaches to — and because the settings tab has to + * be able to report it. + */ +export type PatchResult = + | { installed: true; uninstall: () => void; } + | { installed: false; reason: string; }; + +/** + * 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. + */ +export function installFluentTitles(app: App, options: TransformOptions): PatchResult { + try { + const builtin = findLinkSuggest(app); + if (builtin === null) { + return { installed: false, reason: "the built-in link suggester was not found" }; + } + + const prototype = builtin.constructor?.prototype; + if (typeof prototype?.selectSuggestion !== "function") { + return { installed: false, reason: "the link suggester has no selectSuggestion to wrap" }; + } + + // The patch goes on the located instance's own prototype, never on `EditorSuggest`'s. That one + // is shared with the tag suggester, the footnote suggester, and every suggester every other + // plugin has registered — patching it would have Fluidity inspecting suggestions from surfaces + // it knows nothing about. + // + // `monkey-around` rather than a hand-rolled wrapper because it composes: several plugins wrap + // this same method, and its uninstaller is written so that removing one out of order does not + // strand the others. + const uninstall = around(prototype, { + selectSuggestion: (old: SelectSuggestion): SelectSuggestion => + function(this: LinkSuggest, item: LinkSuggestion, evt: MouseEvent | KeyboardEvent): void { + // Read before delegating. The original calls `this.close()` as its first statement, and + // `close()` clears `this.context` — so reading it afterwards reads null. This is the + // kind of thing that works perfectly against a fake and fails on the first keystroke. + const context = this.context; + + let chosen = item; + try { + 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 + // plugin. + console.error("Fluidity: adjusting a completion failed, inserting it unchanged", error); + } + + return old.call(this, chosen, evt); + }, + }); + + return { installed: true, uninstall }; + } catch (error) { + return { installed: false, reason: `installing the completer patch failed: ${String(error)}` }; + } +} + +/** + * Locate the built-in link suggester **by capability**. + * + * `app.workspace.editorSuggest` holds `suggests: EditorSuggest[]`, and dispatch is first non-null + * `onTrigger` wins, in array order. The built-in is `suggests[0]` at startup, but plugins + * `unshift` their own ahead of it, so an index is unsafe. + * + * A class name is worse than unsafe: in a release build the minified name is literally `"t"`, so a + * filter testing it is a silent no-op that never fires and never complains. At least one published + * plugin has exactly that bug. `suggestManager` is a property only the link suggester carries. + */ +function findLinkSuggest(app: App): LinkSuggest | null { + const registry = (app.workspace as unknown as { editorSuggest?: { suggests?: unknown[]; }; }).editorSuggest; + const suggests = registry?.suggests; + if (!Array.isArray(suggests)) return null; + + const builtin = suggests.find((candidate) => + typeof candidate === "object" && candidate !== null && "suggestManager" in candidate + ); + + return (builtin as LinkSuggest | undefined) ?? null; +} diff --git a/src/suggest/transform.ts b/src/suggest/transform.ts new file mode 100644 index 0000000..a851ee9 --- /dev/null +++ b/src/suggest/transform.ts @@ -0,0 +1,95 @@ +/** + * The decision: which suggestion should Obsidian be asked to insert? + * + * This is the seam the whole plugin is arranged around. Everything above it is about reaching the + * completer at all; everything below it is pure. The module itself is thin and delegating on + * purpose — it decides whether a suggestion is ours to touch, gathers the three facts the rule + * needs, and hands off. + * + * It imports Obsidian for types only, which TypeScript erases, so it carries no runtime edge to + * the app and is exercised from `test/unit` against ordinary objects. + * + * Nothing here builds link text. Obsidian's own composer turns a suggestion into a link, and it + * already knows about Wikilinks-versus-Markdown and every other setting a user may have moved; + * adjusting the suggestion inherits all of that, and keeps the insertion a single transaction. + */ + +import type { App, EditorSuggestContext } from "obsidian"; + +import { fluentDisplay } from "../fluent/display"; +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. */ +export interface TransformOptions { + /** The frontmatter property that marks a note fluent. */ + property: string; +} + +/** The `[[` that opened the completion, which sits immediately before the context's start. */ +const LINK_OPENER_LENGTH = 2; + +/** What turns that opener into an embed, `![[`, rather than a link. */ +const EMBED_MARKER = "!"; + +/** + * Adjust a suggestion on its way to Obsidian's composer, or return it untouched. + * + * Returning the argument **by identity** is how "not ours" is expressed: an unhandled type, a note + * that is not fluent, a link at a sentence start, and a transform that would change nothing all + * take that path. The last of those is what stops an already-lowercase title from producing + * `[[interiority|interiority]]`. + * + * `app` is here because fluency is the vault's own data, read from the metadata cache rather than + * by parsing a file. Keeping that read on this side of the seam leaves `suggest/patch` with no + * decisions of its own to make. + */ +export function transformSuggestion( + app: App, + item: LinkSuggestion, + context: EditorSuggestContext | null, + options: TransformOptions, +): LinkSuggestion { + if (!isAliasSuggestion(item) && !isFileSuggestion(item)) return item; + + // Without the context there is no cursor, so there is no way to tell a sentence start from the + // middle of a clause. Declining to guess leaves the completer behaving exactly as it does + // without the plugin, which is the right answer to not knowing. + if (context === null) return item; + + // The same completer serves `![[`, and an embed is not a link in running text: it renders the + // note, and what follows the pipe there is a display argument rather than prose. The `!` left + // sitting before the opener is the only thing that tells the two apart. + const before = textBeforeLink(context); + if (before.endsWith(EMBED_MARKER)) return item; + + const frontmatter = app.metadataCache.getFileCache(item.file)?.frontmatter; + if (!isFluent(frontmatter, options.property)) return item; + + // An alias item carries the text that will follow the `|`; a file item has none, because the + // composer derives the display text from the note's own title. + const displayText = item.type === "alias" ? item.alias : item.file.basename; + const fluent = fluentDisplay(displayText, startsSentence(before)); + if (fluent === displayText) return item; + + // The item belongs to Obsidian's suggester and may well be reused, so it is shallow-cloned + // before anything is changed. + // + // A file item becomes an alias item carrying the fluent form. There is no display text on a file + // item to adjust, and the composer sets `alias = display = item.alias` for an alias item without + // collapsing case — which is the entire fluent-titles feature, and is the one thing + // `generateMarkdownLink` will not do. + return { ...item, type: "alias", alias: fluent }; +} + +/** + * The text on the line up to the `[[` being completed. + * + * This comes off the completer's own context, which is a real advantage of patching it: every + * other route to this feature has to infer what was just typed from a diff of the document. + */ +function textBeforeLink(context: EditorSuggestContext): string { + const line = context.editor.getLine(context.start.line); + return line.slice(0, Math.max(0, context.start.ch - LINK_OPENER_LENGTH)); +} diff --git a/test/integration/suggest/patch.test.ts b/test/integration/suggest/patch.test.ts new file mode 100644 index 0000000..e9aed83 --- /dev/null +++ b/test/integration/suggest/patch.test.ts @@ -0,0 +1,249 @@ +/** + * The patch, against a fake suggester built to match what Obsidian's is believed to look like. + * + * `suggest/item` writes down the shapes and `suggest/transform` is tested against plain objects; + * neither exercises the part that reaches into the app. This suite does, by standing up a + * suggester registry with the same shape the real one has and installing the real patch into it. + * + * It cannot catch Obsidian changing — nothing outside a vault can. What it catches is *us* + * changing: every assertion here corresponds to a rule `suggest/patch` states in a comment, and + * each of those rules is one a plausible refactor would quietly break while `test/unit` stayed + * green. The sharpest is the context rule, because getting it wrong produces code that works + * perfectly against a naive fake — one whose `selectSuggestion` leaves `context` alone — and fails + * on the first real keystroke. + */ + +import assert from "node:assert/strict"; +import test, { type TestContext } from "node:test"; + +import type { App } from "obsidian"; + +import type { LinkSuggestion } from "../../../src/suggest/item.ts"; +import { installFluentTitles } from "../../../src/suggest/patch.ts"; + +const OPTIONS = { property: "fluent" }; + +/** Stands in for the `TFile` on a suggestion. */ +const file: any = { basename: "Interiority", path: "Interiority.md" }; + +const fileItem = (): any => ({ type: "file", file, path: "Interiority.md" }); + +/** A completer context for a link typed after `before`, with `before` the line until `[[`. */ +function contextFor(before: string): any { + const line = `${before}[[`; + return { editor: { getLine: () => line }, start: { line: 0, ch: line.length } }; +} + +/** + * The base every suggester in the app shares, standing in for `EditorSuggest`. + * + * Its `selectSuggestion` clears the context first, exactly as Obsidian's does by calling + * `this.close()`, because that single detail is what the patch has to be written around. + */ +class EditorSuggest { + context: any = null; + received: { item: LinkSuggestion; evt: unknown; }[] = []; + + selectSuggestion(item: LinkSuggestion, evt: unknown): void { + this.context = null; + this.received.push({ item, evt }); + } +} + +/** The built-in link suggester: its own subclass, carrying the property the patch looks for. */ +class LinkSuggest extends EditorSuggest { + suggestManager = {}; +} + +/** Any other suggester on that same base — the tag suggester, or one belonging to a plugin. */ +class OtherSuggest extends EditorSuggest {} + +/** A registry holding `suggests` in the order Obsidian would, plus a metadata cache. */ +function appWith(suggests: unknown[], frontmatter: unknown = { fluent: true }): App { + return { + workspace: { editorSuggest: { suggests } }, + metadataCache: { getFileCache: () => ({ frontmatter }) }, + } as unknown as App; +} + +/** + * Install into `app`, and take the patch back off when the test ends. + * + * The patch goes on a class prototype, which every test in this file shares. An install that + * 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); + + assert.equal(result.installed, true, "expected the fake registry to be patchable"); + t.after(() => { + if (result.installed) result.uninstall(); + }); + + return result; +} + +/** Install into a fresh registry, returning everything a test needs to drive it. */ +function install(t: TestContext, before = "about the ", frontmatter: unknown = { fluent: true }) { + const suggest = new LinkSuggest(); + const other = new OtherSuggest(); + // A plugin's own suggester sits ahead of the built-in, which is why an index is unsafe. + const app = appWith([other, suggest], frontmatter); + const result = installFor(t, app); + + suggest.context = contextFor(before); + // The decoy needs one too. Without it the transform would decline to guess and hand the item + // back by identity, so every "the decoy is left alone" assertion would hold even if the patch + // had gone on the shared base. + other.context = contextFor(before); + + return { suggest, other, result }; +} + +test("a fluent title is lowercased on its way to the composer", (t) => { + const { suggest } = install(t); + const evt = { type: "keydown" }; + + suggest.selectSuggestion(fileItem(), evt); + + const inserted: any = suggest.received[0]?.item; + assert.equal(inserted.type, "alias"); + assert.equal(inserted.alias, "interiority"); + assert.equal(inserted.file, file, "the note must travel with the suggestion"); +}); + +test("the context is taken from the live suggester on every call", (t) => { + // The fake clears `context` as its first statement, exactly as `close()` does. + // + // Statement order inside the wrapper is not what this pins — it cannot go wrong, because the + // adjusted item is an argument to the original call and so has to be computed first. What can go + // wrong is *where* the context is read from: a wrapper that captured it when the patch was + // installed, or read it off anything but the instance being called, sees the cleared value and + // silently stops adjusting anything at all. That is what the assertion below catches. + const { suggest } = install(t, "about the "); + + suggest.selectSuggestion(fileItem(), {}); + + assert.equal(suggest.context, null, "the fake must really have cleared it"); + assert.equal((suggest.received[0]?.item as any).alias, "interiority"); +}); + +test("the suggester is found by capability rather than by position", (t) => { + // `suggests[0]` is another plugin's here. Finding the built-in by index would patch that one. + const { suggest, other } = install(t); + + suggest.selectSuggestion(fileItem(), {}); + other.selectSuggestion(fileItem(), {}); + + assert.equal((suggest.received[0]?.item as any).type, "alias"); + assert.equal((other.received[0]?.item as any).type, "file", "the decoy must be left alone"); +}); + +test("only the link suggester's own prototype is patched", (t) => { + // Patching the shared base would have Fluidity inspecting suggestions from every surface in the + // app, including ones belonging to other plugins. The base is read through its own property + // descriptor rather than off the prototype, so that what is compared is the function defined + // there, not whatever the subclass may now be inheriting or shadowing. + const stockOf = (o: object): unknown => Object.getOwnPropertyDescriptor(o, "selectSuggestion")?.value; + const stock = stockOf(EditorSuggest.prototype); + const { other } = install(t); + + assert.equal(stockOf(EditorSuggest.prototype), stock, "the shared base must be untouched"); + assert.ok(Object.hasOwn(LinkSuggest.prototype, "selectSuggestion"), "the patch belongs on the subclass"); + + other.selectSuggestion(fileItem(), {}); + assert.equal((other.received[0]?.item as any).type, "file"); +}); + +test("a suggestion type Fluidity does not handle arrives by identity", (t) => { + // A block completion writes a block id into the target note. Handing the original the same + // object it was going to get is the only safe thing to do with one. + const { suggest } = install(t); + const item: any = { type: "block", file, path: "Interiority.md" }; + + suggest.selectSuggestion(item, {}); + + assert.equal(suggest.received[0]?.item, item); +}); + +test("the event is handed on untouched", (t) => { + // `evt` may be a MouseEvent or a KeyboardEvent, and the original needs the one it was given. + const { suggest } = install(t); + const evt = { which: 13 }; + + suggest.selectSuggestion(fileItem(), evt); + + assert.equal(suggest.received[0]?.evt, evt); +}); + +test("a note that is not fluent reaches the composer exactly as it left the popup", (t) => { + const { suggest } = install(t, "about the ", { fluent: false }); + const item = fileItem(); + + suggest.selectSuggestion(item, {}); + + assert.equal(suggest.received[0]?.item, item); +}); + +test("uninstalling puts the completer back", (t) => { + const { suggest, result } = install(t); + assert.equal(result.installed, true); + if (!result.installed) return; + + result.uninstall(); + const item = fileItem(); + suggest.context = contextFor("about the "); + suggest.selectSuggestion(item, {}); + + assert.equal(suggest.received[0]?.item, item, "the wrapper should be gone, not merely inert"); +}); + +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. + const registries: unknown[] = [ + {}, + { workspace: {} }, + { workspace: { editorSuggest: {} } }, + { workspace: { editorSuggest: { suggests: "not an array" } } }, + { workspace: { editorSuggest: { suggests: [{ noSuggestManager: true }] } } }, + { workspace: { editorSuggest: { suggests: [{ suggestManager: {} }] } } }, + ]; + + for (const app of registries) { + 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/); + } +}); + +test("a transform that throws still inserts what Obsidian handed over", (t) => { + // The keystroke belongs to the user. A failure to adjust a suggestion is not a reason to drop + // one, so the wrapper falls through with the original item and says so once. + const suggest = new LinkSuggest(); + const app = { + workspace: { editorSuggest: { suggests: [suggest] } }, + metadataCache: { + getFileCache: () => { + throw new Error("metadata cache exploded"); + }, + }, + } as unknown as App; + + installFor(t, app); + suggest.context = contextFor("about the "); + + const item = fileItem(); + const logged: unknown[] = []; + const restore = console.error; + console.error = (...args: unknown[]) => logged.push(args); + try { + suggest.selectSuggestion(item, {}); + } finally { + console.error = restore; + } + + assert.equal(suggest.received[0]?.item, item); + assert.equal(logged.length, 1); +}); diff --git a/test/unit/fluent/display.test.ts b/test/unit/fluent/display.test.ts new file mode 100644 index 0000000..e1d2b8e --- /dev/null +++ b/test/unit/fluent/display.test.ts @@ -0,0 +1,30 @@ +/** + * The casing rule. + * + * Small enough to read in one go, and the single place every casing decision passes through — so + * these cases are what the roadmap's override language will have to keep answering. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { fluentDisplay } from "../../../src/fluent/display.ts"; + +test("a fluent title mid-sentence is lowercased whole", () => { + assert.equal(fluentDisplay("Interiority", false), "interiority"); + assert.equal(fluentDisplay("Object Oriented Programming", false), "object oriented programming"); +}); + +test("a fluent title at a sentence start keeps its capital", () => { + assert.equal(fluentDisplay("Interiority", true), "Interiority"); +}); + +test("the rule is blunt, and that is the documented behavior", () => { + // A genuine proper noun is over-lowered. The workaround is not marking such a note fluent, and + // the fix is the roadmap's per-note override language rather than a dictionary. + assert.equal(fluentDisplay("History of France", false), "history of france"); +}); + +test("lowercasing is locale-aware rather than ASCII", () => { + assert.equal(fluentDisplay("ÉTAT", false), "état"); +}); diff --git a/test/unit/fluent/frontmatter.test.ts b/test/unit/fluent/frontmatter.test.ts new file mode 100644 index 0000000..948b89f --- /dev/null +++ b/test/unit/fluent/frontmatter.test.ts @@ -0,0 +1,37 @@ +/** + * What marks a note fluent. + * + * The strictness about booleans is the whole of this module's behavior, and it is a deliberate + * choice rather than an accident of how the value is read, so it is pinned here. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { isFluent } from "../../../src/fluent/frontmatter.ts"; + +test("a real boolean true marks the note", () => { + assert.equal(isFluent({ fluent: true }, "fluent"), true); +}); + +test("a string does not mark the note", () => { + // Obsidian's property editor writes a real boolean for a checkbox property, so a string is a + // typo — and a typo must not change how links are written. + assert.equal(isFluent({ fluent: "true" }, "fluent"), false); + assert.equal(isFluent({ fluent: "yes" }, "fluent"), false); +}); + +test("false and absent are the same thing", () => { + assert.equal(isFluent({ fluent: false }, "fluent"), false); + assert.equal(isFluent({ title: "Interiority" }, "fluent"), false); +}); + +test("a note with no frontmatter at all is not fluent", () => { + assert.equal(isFluent(undefined, "fluent"), false); + assert.equal(isFluent(null, "fluent"), false); +}); + +test("the property name is the caller's to choose", () => { + assert.equal(isFluent({ "common-noun": true }, "common-noun"), true); + assert.equal(isFluent({ "common-noun": true }, "fluent"), false); +}); diff --git a/test/unit/prose/sentence.test.ts b/test/unit/prose/sentence.test.ts new file mode 100644 index 0000000..46d3e69 --- /dev/null +++ b/test/unit/prose/sentence.test.ts @@ -0,0 +1,110 @@ +/** + * What the sentence detector counts as a sentence start. + * + * This is where "it did the wrong thing" usually comes from, so the cases below are the feature + * reference's own table, written out one by one. The cases at the end are the documented + * simplifications: they assert what the detector actually does, which is to answer `true` and + * leave a capital alone, rather than what perfect English would say. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { startsSentence } from "../../../src/prose/sentence.ts"; + +test("a link that opens the line starts a sentence", () => { + assert.equal(startsSentence(""), true); + assert.equal(startsSentence(" "), true); +}); + +test("Markdown structure before the link starts a sentence", () => { + assert.equal(startsSentence("## "), true); + assert.equal(startsSentence("> "), true); + assert.equal(startsSentence(">> "), true); + assert.equal(startsSentence("> [!note] "), true); + assert.equal(startsSentence("- "), true); + assert.equal(startsSentence("* "), true); + assert.equal(startsSentence("+ "), true); + assert.equal(startsSentence("1. "), true); + assert.equal(startsSentence("- [ ] "), true); + assert.equal(startsSentence("- [x] "), true); +}); + +test("terminating punctuation followed by whitespace starts a sentence", () => { + assert.equal(startsSentence("It ended. "), true); + assert.equal(startsSentence("Really! "), true); + assert.equal(startsSentence("Did it? "), true); + assert.equal(startsSentence("Note: "), true); + assert.equal(startsSentence("Then; "), true); +}); + +test("structure inside a blockquote or callout still opens a sentence", () => { + // A bullet in a quote is a bullet. Missing these lowercases a title at the start of a line, + // which is the direction this module is built not to fail in. + assert.equal(startsSentence("> - "), true); + assert.equal(startsSentence("> - [ ] "), true); + assert.equal(startsSentence("> 1. "), true); + assert.equal(startsSentence("> ## "), true); + assert.equal(startsSentence("> [!note] - "), true); + assert.equal(startsSentence(">> - "), true); +}); + +test("a sentence that ends inside quotes or brackets has still ended", () => { + assert.equal(startsSentence("He said \"It ends.\" "), true); + assert.equal(startsSentence("(see above.) "), true); + assert.equal(startsSentence("«C’est fini.» "), true); +}); + +test("a table cell opens a sentence", () => { + // The cell is the unit of prose in a table, so a title first in one keeps its capital. The + // cells before it on the row are prose of their own and do not change that. + assert.equal(startsSentence("| "), true); + assert.equal(startsSentence("|"), true); + assert.equal(startsSentence("| a | "), true); +}); + +test("the CJK terminators need no space after them", () => { + assert.equal(startsSentence("終わり。"), true); + assert.equal(startsSentence("本当！"), true); + assert.equal(startsSentence("どう？"), true); +}); + +test("a terminator with nothing after it does not start a sentence", () => { + // `end.[[Note]]` is someone typing a link onto the end of a word, not a new sentence. + assert.equal(startsSentence("end."), false); +}); + +test("emphasis and opening brackets are transparent", () => { + assert.equal(startsSentence("**"), true); + assert.equal(startsSentence("_"), true); + assert.equal(startsSentence("=="), true); + assert.equal(startsSentence("(\""), true); + assert.equal(startsSentence("- **"), true); + assert.equal(startsSentence("Note: **"), true); +}); + +test("emphasis does not make mid-sentence text into a sentence start", () => { + // The markers are dropped, and what they were attached to is still mid-clause. + assert.equal(startsSentence("a really *"), false); + assert.equal(startsSentence("**bold** "), false); +}); + +test("ordinary prose before the link is not a sentence start", () => { + assert.equal(startsSentence("which is really about the "), false); + assert.equal(startsSentence("an introduction to "), false); + assert.equal(startsSentence("see "), false); +}); + +test("a full stop ending the text is taken at face value", () => { + // An abbreviation, a URL or a span of inline code that ends immediately before the link reads as + // a boundary here, so the capital stays. That is the conservative failure, and a visible one. + assert.equal(startsSentence("e.g. "), true); + assert.equal(startsSentence("see https://example.com/tides. "), true); +}); + +test("a full stop inside the text is not a boundary", () => { + // The rule only ever looks at what the text ends with, so a decimal or an abbreviation earlier + // in the clause costs nothing: these are correctly mid-sentence. + assert.equal(startsSentence("3.14 is the "), false); + assert.equal(startsSentence("e.g. the "), false); +}); diff --git a/test/unit/suggest/transform.test.ts b/test/unit/suggest/transform.test.ts new file mode 100644 index 0000000..909f11b --- /dev/null +++ b/test/unit/suggest/transform.test.ts @@ -0,0 +1,132 @@ +/** + * The decision, exercised against ordinary objects. + * + * `suggest/transform` imports Obsidian for types only, which TypeScript erases, so it loads under + * plain Node and the fakes below are enough to drive every path through it. What cannot be checked + * here is that Obsidian's real objects look like these — that belief lives in `suggest/item` and + * is checked in a vault, by hand. + * + * Passing an item through **by identity** is the contract for "not ours to touch", so these + * assertions compare references rather than shapes wherever that is what is being claimed. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import type { App, EditorSuggestContext } from "obsidian"; + +import type { AliasSuggestion, FileSuggestion, LinkSuggestion } from "../../../src/suggest/item.ts"; +import { transformSuggestion } from "../../../src/suggest/transform.ts"; + +const OPTIONS = { property: "fluent" }; + +/** Stands in for the `TFile` on a suggestion; the transform reads only its basename. */ +const file: any = { basename: "Interiority", path: "Interiority.md" }; + +/** An app whose metadata cache reports exactly this frontmatter for every note. */ +function appWith(frontmatter: unknown): App { + return { metadataCache: { getFileCache: () => ({ frontmatter }) } } as unknown as App; +} + +/** A completer context for a link typed after `before`, with `before` the whole line until `[[`. */ +function contextFor(before: string): EditorSuggestContext { + const line = `${before}[[`; + return { + editor: { getLine: () => line }, + start: { line: 0, ch: line.length }, + } as unknown as EditorSuggestContext; +} + +const aliasItem = (alias: string): AliasSuggestion => ({ type: "alias", alias, file, path: "Interiority.md" }); +const fileItem = (): FileSuggestion => ({ type: "file", file, path: "Interiority.md" }); + +test("an alias for a fluent note is lowercased mid-sentence", () => { + const item = aliasItem("Interior Life"); + const result = transformSuggestion(appWith({ fluent: true }), item, contextFor("a question about the "), OPTIONS); + + assert.deepEqual(result, { type: "alias", alias: "interior life", file, path: "Interiority.md" }); +}); + +test("an alias for a fluent note keeps its capital at a sentence start", () => { + const item = aliasItem("Interior Life"); + const result = transformSuggestion(appWith({ fluent: true }), item, contextFor("Note: "), OPTIONS); + + assert.equal(result, item); +}); + +test("a note's own title becomes an alias item carrying the fluent form", () => { + // There is no display text on a file item to adjust, because the composer derives it from the + // path — so the item is flipped to the type that does carry one. + const item = fileItem(); + const result = transformSuggestion(appWith({ fluent: true }), item, contextFor("really about the "), OPTIONS); + + assert.deepEqual(result, { type: "alias", alias: "interiority", file, path: "Interiority.md" }); +}); + +test("a note that is not fluent is passed through by identity", () => { + const item = aliasItem("Interior Life"); + const context = contextFor("a question about the "); + + assert.equal(transformSuggestion(appWith({}), item, context, OPTIONS), item); + assert.equal(transformSuggestion(appWith({ fluent: false }), item, context, OPTIONS), item); + assert.equal(transformSuggestion(appWith({ fluent: "true" }), item, context, OPTIONS), item); + assert.equal(transformSuggestion(appWith(undefined), item, context, OPTIONS), item); +}); + +test("nothing redundant is inserted", () => { + // Lowercasing an already-lowercase title changes nothing, so the item goes through untouched + // rather than becoming `[[interiority|interiority]]`. + const item = aliasItem("interior life"); + const result = transformSuggestion(appWith({ fluent: true }), item, contextFor("about the "), OPTIONS); + + assert.equal(result, item); +}); + +test("suggestion types Fluidity does not handle are passed through by identity", () => { + // A block item writes a block id into the target note when it is selected. Returning anything + // other than the object handed over is how a plugin corrupts a file. + const context = contextFor("about the "); + const app = appWith({ fluent: true }); + + for (const type of ["block", "heading", "linktext", "none"]) { + const item = { type } as LinkSuggestion; + assert.equal(transformSuggestion(app, item, context, OPTIONS), item); + } +}); + +test("a malformed item is passed through rather than read", () => { + const app = appWith({ fluent: true }); + const context = contextFor("about the "); + + const items = [{ type: "alias" }, { type: "file" }, { type: "alias", file }, { type: "file", file: {} }]; + for (const item of items as LinkSuggestion[]) { + assert.equal(transformSuggestion(app, item, context, OPTIONS), item); + } +}); + +test("an embed is passed through by identity", () => { + // `![[Note]]` renders the note rather than reading as prose, and the text after a pipe there is + // a display argument. The completer is the same one, so the `!` is all there is to go on. + const app = appWith({ fluent: true }); + + for (const item of [aliasItem("Interior Life"), fileItem()]) { + assert.equal(transformSuggestion(app, item, contextFor("about the !"), OPTIONS), item); + assert.equal(transformSuggestion(app, item, contextFor("!"), OPTIONS), item); + } +}); + +test("a completion with no context is passed through by identity", () => { + // Without a cursor there is no way to tell a sentence start from mid-clause, and declining to + // guess leaves the completer behaving exactly as it does without the plugin. + const item = aliasItem("Interior Life"); + + assert.equal(transformSuggestion(appWith({ fluent: true }), item, null, OPTIONS), item); +}); + +test("the item Obsidian handed over is never mutated", () => { + // It belongs to the suggester and may well be reused, so the transform clones before changing. + const item = aliasItem("Interior Life"); + transformSuggestion(appWith({ fluent: true }), item, contextFor("about the "), OPTIONS); + + assert.equal(item.alias, "Interior Life"); +});