FR: Trigger custom events on linting

baodrate commented 2 weeks ago

Is Your Feature Request Related to a Problem? Please Describe.

Currently, the only way to add custom behavior when files are linted is to set a custom command. obsidian-linter will call this command after a file is linted. The consequences of this approach:

files are not passed directly to the custom command. custom commands are expected to operate on workspace.getActiveFile().
- the current approach is to open the file in a sidebar tab before calling the command. the downside of this is that it is quite slow and fragile
- quoting the current code: "Linting multiple files with custom commands enabled is a slow process that requires the ability to open panes in the side panel. It is noticeably slower than running without custom commands enabled"
custom commands are always run after linting is complete
- they do not affect the diff reporting, and multiple linter runs might be required to properly format the file

Describe the Solution You'd Like

Trigger custom events prior to and after files are linted, allowing users to define custom lint functions simply by subscribing to the events

Examples of plugins that expose events for other plugins to subscribe to:

Please include an example where applicable:

Example implementation

Adding event triggers to obsidian-linter

```diff diff --git a/src/main.ts b/src/main.ts index 923661e..a161bd3 100644 --- a/src/main.ts +++ b/src/main.ts @@ -433,11 +433,19 @@ export default class LinterPlugin extends Plugin { } async runLinterFile(file: TFile, lintingLastActiveFile: boolean = false) { + this.app.workspace.trigger('linter:pre-lint-file', file); const oldText = stripCr(await this.app.vault.read(file)); + const lintInfo = { + file: file, + contents: oldText, + }; + this.app.workspace.trigger('linter:lint-begin', lintInfo); const newText = this.rulesRunner.lintText(createRunLinterRulesOptions(oldText, file, this.momentLocale, this.settings, this.defaultAutoCorrectMisspellings)); + this.app.workspace.trigger('linter:lint-end', lintInfo); if (oldText != newText) { await this.app.vault.modify(file, newText); + this.app.workspace.trigger('linter:post-lint-file', file, oldText); if (lintingLastActiveFile) { const message = getTextInLanguage('logs.file-change-lint-message-start') + ' ' + this.lastActiveFile.path; @@ -454,6 +462,7 @@ export default class LinterPlugin extends Plugin { return; } + this.app.workspace.trigger('linter:post-lint-file', file, oldText); await this.runCustomCommandsInSidebar(file); } @@ -523,21 +532,27 @@ export default class LinterPlugin extends Plugin { logInfo(getTextInLanguage('logs.linter-run')); + this.app.workspace.trigger('linter:pre-lint-editor', editor); const file = this.app.workspace.getActiveFile(); const oldText = editor.getValue(); - let newText: string; + const lintInfo = { + file: file, + contents: oldText, + }; + this.app.workspace.trigger('linter:lint-begin', lintInfo); try { - newText = this.rulesRunner.lintText(createRunLinterRulesOptions(oldText, file, this.momentLocale, this.settings, this.defaultAutoCorrectMisspellings)); + lintInfo.contents = this.rulesRunner.lintText(createRunLinterRulesOptions(oldText, file, this.momentLocale, this.settings, this.defaultAutoCorrectMisspellings)); } catch (error) { this.handleLintError(file, error, getTextInLanguage('commands.lint-file.error-message') + ' \'{FILE_PATH}\'', false); return; } - - const changes = this.updateEditor(oldText, newText, editor); + this.app.workspace.trigger('linter:lint-end', lintInfo); + const changes = this.updateEditor(oldText, lintInfo.contents, editor); const charsAdded = changes.map((change) => change[0] == DiffMatchPatch.DIFF_INSERT ? change[1].length : 0).reduce((a, b) => a + b, 0); const charsRemoved = changes.map((change) => change[0] == DiffMatchPatch.DIFF_DELETE ? change[1].length : 0).reduce((a, b) => a + b, 0); this.displayChangedMessage(charsAdded, charsRemoved); + this.app.workspace.trigger('linter:post-lint-editor', editor, oldText); // run custom commands now since no change was made if (!charsAdded && !charsRemoved) { @@ -546,7 +561,7 @@ export default class LinterPlugin extends Plugin { this.editorLintFiles.push(file); } - this.updateFileDebouncerText(file, newText); + this.updateFileDebouncerText(file, lintInfo.contents); setCollectLogs(false); } ```

custom lint function using event subscribers

This uses [obsidian-custom-js](https://github.com/saml-dev/obsidian-custom-js). ```javascript class ExtraLintEvent { constructor() { this.app = window.customJS.app; this.preLintFileHandler = this.preLintFileHandler.bind(this); this.postLintFileHandler = this.postLintFileHandler.bind(this); this.lintBeginHandler = this.lintBeginHandler.bind(this); } async invoke() { this.app.workspace.on('linter:pre-lint-file', this.preLintFileHandler); this.app.workspace.on('linter:post-lint-file', this.postLintFileHandler); this.app.workspace.on('linter:lint-begin', this.lintBeginHandler); } deconstructor() { this.app.workspace.off('linter:pre-lint-file', this.preLintFileHandler); this.app.workspace.off('linter:post-lint-file', this.postLintFileHandler); this.app.workspace.off('linter:lint-begin', this.lintBeginHandler); } preLintFileHandler(file) { const app = window.customJS.app; app.fileManager.processFrontMatter(file, (frontmatter) => { frontmatter['foo'] = "baz"; }); } postLintFileHandler(file, oldText) { const app = window.customJS.app; const newText = this.app.vault.read(file) if (oldText !== newText) { app.fileManager.processFrontMatter(file, (frontmatter) => { frontmatter['linted'] = true; }); } } lintBeginHandler(info) { info.contents = info.contents.replace(/foo/g, 'bar'); } } ```

Describe Alternatives You've Considered

Use the custom commands setting
- downsides described in first section
- cannot run prior to linting
Using default file create/modify events
- not specific enough (to only linting actions)
- also cannot run prior to linting

Additional Context

The example implementation I provided adds these events:

linter:post-lint-file
linter:post-lint-editor
linter:lint-begin
linter:lint-end
linter:pre-lint-file
linter:pre-lint-editor

I believe they cover most of the possible use cases I can envision.

linter:pre-lint-file / linter:pre-lint-editor
- callback signatures: (file: TFile) => void / (editor: Editor) => void
- prior to this.app.vault.read(file) / editor.getValue()
linter:lint-begin
- callback signature: (info: { file: TFile, contents: string }) => void
- after reading file/editor, prior to builtin lints
linter:lint-end
- callback signature: (info: { file: TFile, contents: string }) => void
- after builtin lints, prior to writing
linter:post-lint-file / linter:post-lint-editor
- callback signatures: (file: TFile, oldText: string) => void / (editor: Editor, oldText: string) => void
- after linting and writing

The lint-begin and lint-end events act as hooks to allow users to implement custom linting steps, which run either before or after all of the builtin steps

The (pre|post)-lint-(file|editor) events are intended to allow users to perform some operations outside of the chain of linting operations (all linting should be pending/completed at these points)

pjkaufman commented 2 weeks ago

Hey @baodrate . Thank you for the feature request. I think this is a good start to a discussion. I am not 100% sure if the Linter is ready for this kind of functionality yet since most of the linting rules are planned to be moved to a web worker. Would doing so cause problems with this proposed eventing system?

If so, do you have any ideas about mitigating this?

baodrate commented 2 weeks ago

Would doing so cause problems with this proposed eventing system?

This is perhaps a premature answer (I've only skimmed #1047), but I'm inclined to say that it shouldn't. Or more precisely, the change of offloading the data-intensive work onto webworker threads should not make implementing the event triggers any more difficult. However, since the workers cannot directly interact with obsidian's event system, the events would still be run in the main thread (which shouldn't be too much of an issue, IMO, since it'd be a purely opt-in advanced feature)

As for the implementation, the only thing that should change is where the events are triggered.

So for https://github.com/pjkaufman/obsidian-linter/blob/inline-worker/src/rules-runner/file-lint-manager.ts

something like this, perhaps:

--- foo1.js 2024-11-06 05:16:17
+++ foo2.js 2024-11-06 05:26:35
@@ -42,6 +42,7 @@
            const worker = Worker();
            worker.onmessage = async (resp: any) => {
                await this.finish(resp.data as RunLinterRulesOptions, index);
+               this.app.workspace.trigger('linter:post-lint-file', resp.data);
            };

            this.workers.push(worker);
@@ -82,6 +83,7 @@

        // Queue a new job onto this worker.
        const job = this.lintQueue.shift();
+       this.app.workspace.trigger('linter:pre-lint-file', data);
        if (job !== undefined) {
            this.send(job, index);
        }
@@ -114,6 +116,7 @@
            dateModifiedKey: yamlTimestampOptions.dateModifiedKey,
        });

+       this.app.workspace.trigger('linter:lint-end', data);
        if (this.callbacks.has(data.fileInfo.path)) {
            const callback = this.callbacks.get(data.fileInfo.path);
            this.callbacks.delete(data.fileInfo.path);
@@ -131,6 +134,7 @@
        this.busy[workerId] = true;
        this.vault.read(file).then((oldText: string) => {
            const lintRunnerSettings = createRunLinterRulesOptions(stripCr(oldText), file, this.momentLocale, this.settings);
+           this.app.workspace.trigger('linter:lint-begin', lintRunnerSettings);
            this.workers[workerId].postMessage(lintRunnerSettings);
        });
    }

please correct me if i've misunderstood something

pjkaufman commented 2 weeks ago

That sounds right, but I have not dealt with event triggers and whatnot before. So I will need to see about testing this whenever work happens on it. I would prefer to fix some more bugs then get the web worker logic present before taking a stab at this, but I am so scatterbrained at times I may forget about this ticket.

I want to make it clear, that I am not opposed to this addition. I am just not yet sure of some of the intricacies or edge cases that may exist.

baodrate commented 2 weeks ago

I totally get it. I'll wait until the web worker stuff is more finalized and then I'll revisit this ticket. cheers

platers / obsidian-linter