Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Shell commands plugin #433

Merged
merged 2 commits into from Aug 31, 2021
Merged

Add Shell commands plugin #433

merged 2 commits into from Aug 31, 2021

Conversation

Taitava
Copy link
Contributor

@Taitava Taitava commented Aug 22, 2021

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/Taitava/obsidian-shellcommands

Release Checklist

  • I have tested this on Windows, macOS, and Linux (Tested on Windows and Linux, not Mac cos I don't have it. My repo has an issue about asking for someone to test on Mac.)
  • My GitHub release contains all required files
    • main.js
    • manifest.json
    • styles.css (optional)
  • GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
  • The id in my manifest.json matches the id in the community-plugins.json file.
  • README clearly describes the plugins purpose and provides clear usage instructions.
  • I have added a license in the LICENSE file.

@lishid
Copy link
Collaborator

@lishid lishid commented Aug 26, 2021

https://github.com/Taitava/obsidian-shellcommands/blob/93a46097410111d83ba7dc8988943f042bc7ca1c/main.ts#L9

Should consistently use imports for this, right?

https://github.com/Taitava/obsidian-shellcommands/blob/93a46097410111d83ba7dc8988943f042bc7ca1c/Common.ts#L6
This should be something like

if (adapter instanceof FileSystemAdapter) {
  return adapter.getBasePath();
}
return null;

https://github.com/Taitava/obsidian-shellcommands/blob/93a46097410111d83ba7dc8988943f042bc7ca1c/Common.ts#L14
Various things that can improve here

  • activeLeaf shouldn't be directly used, it can also be null. Instead, you should use workspace.getActiveViewOfType(MarkdownView).
  • Instead of using view.getViewState().state.mode, you can use view.getMode()

https://github.com/Taitava/obsidian-shellcommands/blob/93a46097410111d83ba7dc8988943f042bc7ca1c/ShellCommandVariableParser.ts#L90
Better way here would be to use navigator.clipboard.readText(), although that's an async and returns a promise instead...

https://github.com/Taitava/obsidian-shellcommands/blob/93a46097410111d83ba7dc8988943f042bc7ca1c/ShellCommandVariableParser.ts#L230
This may return null.

@Taitava
Copy link
Contributor Author

@Taitava Taitava commented Aug 26, 2021

Thanks, I will look into these! :)

@Taitava
Copy link
Contributor Author

@Taitava Taitava commented Aug 27, 2021

Better way here would be to use navigator.clipboard.readText(), although that's an async and returns a promise instead...

I tried to learn Promises now, but wasn't succesful to figure out how they work :(. This is something I need to look into later.

Everything else should be fixed now. Please let me know if there's still something to fix :).

P.S. The current released version 0.0.0 is quite behind in some features. I will release a new version (0.1.0) when this plugin is good to be included in the community plugins list. I could do a release also now, but I'd rather wait to see if there's still something to fix. I'll fix the Promise thing later, if it's ok?

Edit: Whoops, tried, not tired :D

@lishid
Copy link
Collaborator

@lishid lishid commented Aug 27, 2021

https://github.com/Taitava/obsidian-shellcommands/blob/19a742c494be9d725b290859d14713118f86b5e4/Common.ts#L19
You'll want to test for truthiness here and return if not.

I tired to learn Promises now, but wasn't succesful to figure out how they work :(. This is something I need to look into later.

Ah yes - promises is the replacement for callbacks, but I think in your case since the rest of your code isn't designed for async processing this may not work very well - since you're targeting the desktop app anyway, using the electron API should be fine.

I am ready to merge this - let me know when you make the release!

@Taitava
Copy link
Contributor Author

@Taitava Taitava commented Aug 27, 2021

https://github.com/Taitava/obsidian-shellcommands/blob/19a742c494be9d725b290859d14713118f86b5e4/Common.ts#L19
You'll want to test for truthiness here and return if not.

So something like this?

if (view_mode) {
    if ("editor" in view) {
        // Good, it exists.
        // @ts-ignore We already know that view.editor exists.
        return view.editor;
    }
}
// Else: either view_mode is false-ish, or view didn't contain editor. Write to log and return null.

I have written this function a bit badly: it checks if the view mode is "source". editor could be returned regardless of what the view mode is (as long as view.editor exists). But currently I'm only using this function for getting text that is selected in the current editor in ShellCommandVariable_Selection and that only seems to work if the view is in "source" mode. Can you give any advice how to get it work in "preview" mode too? editor.somethingSelected() returns false if the view is in "preview" mode even though I have something selected in preview.

And btw what is "live" view mode?

Thanks! :)

I'll let you know when I have the release ready.

@lishid
Copy link
Collaborator

@lishid lishid commented Aug 27, 2021

Oh no, I wasn't clear - you'll want to make sure that the view object isn't null or undefined.

@Taitava
Copy link
Contributor Author

@Taitava Taitava commented Aug 29, 2021

Thanks :). Now I've released version 0.1.0.

@lishid lishid merged commit fc8afdc into obsidianmd:master Aug 31, 2021
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants