FR: Add Support for Custom Commands on Lint Folder and Lint Vault

[pjkaufman]

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

Currently custom commands really only work for linting the current file. Make it so they can work for linting all files and all folders. This will likely require exposing an external variable referencing a file that was just linted.

[pjkaufman]

I need to figure out a good way to do a custom command for a file. It will need to expose an API that can be used by others.

I am thinking that the API would consist of the following:

• Get next file to process: gets the next file to process for the Linter (this would need to be a reference to the file in question that needs to have custom commands on it).

Questions that need answering:

• How does this affect Linting a single file?
• How is running multiple linting actions handled (lint all files in foldeder then lint the current file before folder lint completes)?
• How do we handle files that did not have custom commands run?

[pjkaufman]

I am under the impression that the Linter runs synchronously based on the way that I see results. However it may not run synchronously (i.e. 1 file at a time).

If it does run 1 file at a time, the following should work for adding this:

• Add an internal global array for storing the file identifier to use for updating and getting the file from custom commands
• add the TFile to the global array when running custom commands in the Linter
• pop the TFile from the global array when running custom commands for the Linter has completed
• Uncomment line for using custom commands in multiple files with the Linter
• Add a globally API for the Linter with a method to get the file to process
• Update documentation to reflect this change

[pjkaufman]

Looks like things are not linear so a more complex approach to updates is needed.

[pjkaufman]

Looks like I would need to do the following:

Using an array should also work if you run the commands sequentially. But then since you are only running one command at a time if you do it this way, you only need a TFile instead of a TFile[].

Well, except that you will also need to replace executeCommandById with getting the callback from the Command directly and do:

await callback()

Because executeCommandById does not return a Promise, there is no way it handles async callback properly (i.e. awaiting it).

See https://discord.com/channels/686053708261228577/840286264964022302/1114030007401074788

This would mean that I would have to expose a TFile globably on the Linter and it should allow me to pull that in on QuickAdd without accidentally writting to or updating the same file more than once per obsidian command.

[pjkaufman]

A suggested change to the run method would be found at the following link: https://discord.com/channels/686053708261228577/840286264964022302/1114036798235099166

It is also copied here:

Okay, I guess you can figure out what to do if this happens later.

For now, I think my solution would be to change runCustomCommands to this:

import AsyncLock from "async-lock"

class Runner {
    private lock = new AsyncLock()

    async runCustomCommands(file: TFile, lintCommands: LintCommand[], commands: ObsidianCommandInterface) {
        await this.lock.acquire("command", async () => {
            logDebug(getTextInLanguage('logs.running-custom-lint-command'));
            window.currentFile = file // the user is expected to get the file from this variable
            try {
                const commandsRun = new Set<string>();
                for (const commandInfo of lintCommands) {
                    if (!commandInfo.id) {
                        continue
                    } else if (commandsRun.has(commandInfo.id)) {
                        logWarn(getTextInLanguage('logs.custom-lint-duplicate-warning').replace('{COMMAND_NAME}', commandInfo.name));
                        continue
                    }
                    try {
                        commandsRun.add(commandInfo.id)
                        const cmd = commands.findCommand(commandInfo.id)
                        if (!cmd || !cmd.callback) {
                            throw new Error() // cmd or callback is not found
                        }
                        await cmd.callback()
                    } catch (error) {
                        wrapLintError(error, `${getTextInLanguage('logs.custom-lint-error-message')} ${commandInfo.id}`);
                    }
                }
            } finally {
                window.currentFile = null // unsets it
            }
        })
    }
}

[Mikle-Bond]

Wouldn't it be easier to perform something like "open file in editor -> lint -> run custom commands -> close file"?

For example, here's how it it done by others:

https://github.com/SimplGy/obsidian-open-file-by-magic-date/blob/b01ddd1622bca34ed1e38b9a581d93136b5f81ae/src/main.ts#L103-L136

We could open files somewhere in the sidebar leafs, so that they do not interfere with users' panes/tiles.

The only problem I see: it would probably be slow. But it can be solved with a modal warning, that this process is slow, and then keep showing progress bar until it is done.

Or is the problem somewhere deeper?

[pjkaufman]

The main thing around this is opening files is slow. I don't want to force the opening of files as that messes with the file history and will likely cause conflicts with lint on file change.

So far the best I have been able to think up is running this logic using a mutex to make sure it runs on the right file.

[pjkaufman]

I have to have a way to let commands know which file is being linted. Opening the file would alleviate that, but it would cause the issue of messing up the file history and slow things down greatly.

[pjkaufman]

Wouldn't it be easier to perform something like "open file in editor -> lint -> run custom commands -> close file"?

For example, here's how it it done by others:

https://github.com/SimplGy/obsidian-open-file-by-magic-date/blob/b01ddd1622bca34ed1e38b9a581d93136b5f81ae/src/main.ts#L103-L136

We could open files somewhere in the sidebar leafs, so that they do not interfere with users' panes/tiles.

The only problem I see: it would probably be slow. But it can be solved with a modal warning, that this process is slow, and then keep showing progress bar until it is done.

Or is the problem somewhere deeper?

I am double checking with devs on the discord for Obsidian around this, but I may do this for simplicity's sake if it is not a concern to any of them. Personally this would be something that is only necessary for Custom Commands so it would not need to happen before linting or regex replace. However I personally have thousands of files so linting my vault might go from 30 seconds to something much larger in time.

However this could be a viable solution when a custom command is present.

[pjkaufman]

One potential problem is that custom commands are run asynchronously and might overlapp, so I would still need the async mutex.

[pjkaufman]

So far it seems like the suggestion to open the file with custom commands being run on it in the sidebar/view off to the side seems like it would worl according to other devs.

To combat the loss of file history, the Linter would need to go ahead and iterate over the files found in the file history prior to the linting of multiple files.

During the process, lint on file change would need to be disabled.

[pjkaufman]

Opening a file in the sidebar will require using a leaf as a parent which will need to be retieved something like the following:

this.app.workspace.getRightLeaf(false).setViewState({
      type: COUNTER_VIEW_TYPE,
      active: true,
    });

[Mikle-Bond]

linting my vault might go from 30 seconds to something much larger in time

True, but it is still much faster than opening each file manually. And I would assume it is a "once in millennia" action, performed when you lint an unlinted-yet vault, or update rules (which shouldn't happen often).

Also, I don't think commands are async, because app.commands.executeCommandById is not an async function. Some commands might use something asynchronous inside, such as with setInterval but this would be impossible to catch. I believe this should be users' responsibility to use only synchronous commands, or commands that will not clash.

[pjkaufman]

The more I discuss this, the more I agree with the idea that executeCommandById is synchronous. But with that I still need to add the mutex lock just in case something were to happen where a separate file's custom commands were to try to run before the other rule's custom commands finished. Hopefully that would be a very a small chance of happening, but it would still need to be accounted for. I think I will take a stab at the following approach and see how it works out:

• If a custom command is present when linting a folder or the whole vault, open files in the sidebar while the custom commands for a file are running
• A mutex would be used to ensure that the custom commands are not run incorrectly or from another file on that file
• After all is said and done, it would try to restore the file history of the vault
  • this may be feasible, but this is more of a nice to have, so no guarantees here

[Mikle-Bond]

You might want to look at Webpage Export plugin. It is meant to export rendered views of notes into an HTML pages. But the way it is done: it creates additional leaf, and opens every note in it. It hides the leaf, and replaces it with progress bar source. As a funny side effect, it also lints every exported note if "Lint on focus change" is enabled)

Maybe the same code could be used to create editor view for custom commands?

[pjkaufman]

@Mikle-Bond , would you be willing to test out a custom version of the plugin to see if it works properly for running custom commands on folder and vault lint and give feedback? I have something that may work. But I am not sure if it works in a real use case. I have a test case that interted a new line at the end of files, but that is as far as I am able to test it myself.

[Mikle-Bond]

@pjkaufman yeah, sure. Though I can't guarantee I'll be quick to respond due to work, but I do have a messed vault that I can copy and crush-test) Is is change in master brunch, or is it not committed yet?

[pjkaufman]

It is not committed yet. I can make sure the changes are up to date and then add a reference to the branch and add the zipped main.js file here.

[pjkaufman]

Here is the branch with the changes in question: https://github.com/pjkaufman/obsidian-linter/tree/multiple-file-custom-command-support Here is a zip with the main.js and styles.css: main.zip

[Mikle-Bond]

Sorry for the wait, @pjkaufman . I've checked out the changes.

TLDR: It works, thanks for your hard work! But as I said before, when someone's plugin command wants to be asynchronous -- it will be.

I've tried several plugins with commands that I could think of. Here are results and observations:

Advanced tables -- Format All tables

The plugin that made me involved into this discussion works fine, and formats tables (with exact same quirks as noted before in #827)

Quickshare -- Create Share Link

This plugin has to communicate with the outer world via network, but it uses synchronous method and thus works fine. A bunch of notes gets published. I also tried it with network throttling in DevTools enabled to be sure it works as expected.

Telegraph Share -- Publish to Telegraph

This one exposes faulty behavior. Look: https://github.com/reorx/obsidian-telegraph-publish/blob/539b8fcb82c3985ec29fa84bc6247888f3b362d2/main.ts#L67-L73

They just threw async method in there. So the executeCommandById obtains the Promise and exits, while the actual code executes after it returns. What that means is: plugins tries to upload file of destroyed panel

Maybe there would be a way to mitigate that, by reinventing the wheel reimplementing the function executeCommandById and make it inspect whether the function in callback* is async or not. From what I see after googling, this is impossible. After TypeScript is compiled there is no way to know what is async and what isn't.

Pane Relief

This one affects the current implementation. There's an option to prevent focusing sidebar panes (which usually doesn't work with my main vault, but in test vault it actually worked). This plugin prevents focus change to the linted file. For example, when I lint a folder with 3 files, but have another file open, linting directory causes QuickShare triggered 3 times, and all 3 times it uploads currently open file, not the files being linted.

Conclusion

• The mutex doesn't seem to help against the sneaky commands. It only helps with well-behaving commands.
• My previous claim about callbacks being synchronous is partially debunked. The callback itself is synchronous, but apparently no one prevents developers from putting an async in there.
• The side pane approach is good, but not universal. I wonder if getLeaf("window") would behave better with Pane Relief plugin.

[pjkaufman]

Thanks for taking a look @Mikle-Bond!

Here are my thoughts on the conclusions you mentioned:

• The mutex doesn't seem to help against the sneaky commands. It only helps with well-behaving commands.
• My previous claim about callbacks being synchronous is partially debunked. The callback itself is synchronous, but apparently no one prevents developers from putting an async in there.

There is really nothing that can be done for async commands being hidden by the callback if it does not return the promise. I could try monkey patching it, but I am not sure it is worth the effort to handle that scenario unless the solution is pretty simple and maintainable (especially since I would be reliant on users to let me know if it breaks).

• The side pane approach is good, but not universal. I wonder if getLeaf("window") would behave better with Pane Relief plugin.

There could be a setting under general settings that lets you open the files in the sidebar and on non-mobile it could allow you to decide if you would rather open then in a pop out window instead.

What do you think about this?

[pjkaufman]

I know that we have discussed this ticket on and off again. So I decided to bite the bullet and add the initial implementation. It likely has issues with it that will be discovered over time. But with the help of the community we should be able to tackle those issues as they arise. This should be on master and in the next release.