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

Feature / UseURL: open file in workspace #5

Merged
merged 7 commits into from
Mar 28, 2022

Conversation

ptim
Copy link
Contributor

@ptim ptim commented Mar 28, 2022

Thanks for the plugin!

  • I've documented the requirement to use the full path to code on MacOS Plugin doesn't open my vault in VS Code #3
  • I changed the default template to "Open file" which might be a bit cheeky, but it seems the logical behaviour to me (principle of least surprise). It's in a separate commit, so easy to remove if you disapprove!
  • I've found that UseURL is noticeably faster on my machine, so I added that note to the readme and settings (the default is still child_process.exec)
  • Due to the above, I've added two new checkboxes which facilitate opening the current file in your defined VSCode workspace in the case that you're using UseURL

Additionally, I added a prettier config that matched the original as closely as I could (it wasn't consistent) bc I was fighting the editor. Wasn't clear on how you were building, as I see that your dist is generated by rollup, whereas what's in this repo compiles with esbuild... works a treat, regardless!

HTH, ptim

screenshot - test vault - Obsidian v0 14 2 - 2022-03-28 at 16 01

@NomarCub NomarCub merged commit fd106e0 into NomarCub:master Mar 28, 2022
@NomarCub
Copy link
Owner

Hi! Thank you for the PR.
I don't really now much about building TS / JS. I updated the readme with the info I know. I used VSCode's default formatter until now.

I tried to make some modifications before merging, but fucked something up, so I merged anyway. Do you mind opening another PR with some adjustments?
On Windows, due to the dialog that comes up with URL, the second window.open never manifests. This also means that you can't specify to not open the file currently active in Obsidian. For me it'd make sense if the useWorkspace was a checkable setting, transparent to the user. It'd also be nice if you could use the {{}} variables in the the URL workspace path setting.

@ptim
Copy link
Contributor Author

ptim commented Apr 6, 2022

Thanks NomarCub! Great feedback, thanks - I'll send through another PR hopefully this weekend 👍

@NomarCub
Copy link
Owner

How about this weekend? 😄

@NomarCub
Copy link
Owner

@ptim will you take a look at this?

@NomarCub
Copy link
Owner

Hi @ptim? I'd like to make a new release, and without the fixes I'd rather not include this and revert the changes. Will you be able to come through with a new PR soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants