-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Possible issues with marker target commands #38
Comments
Yeah, probably. This (honestly) was a quick end-of-day feature add to get the functionality built in for the user in issue #35. Ultimately since Obsidian (I believe) does not expose the registered commands in their API, I'm not sure how safe I can really make this functionality. Probably putting a warning on it in the documentation will be enough, referencing your caveats. |
Hmm. Not to turn someone down, but I doubt the usage metaphor. With a marker on a map, upon hovering or clicking, I’d expect "more info", "zoom to it" or "go there", much like your original version offers. But not, probably unexpectedly, or even dangerous, commands being executed, or files created. This may be convenient for a (very) few users, but, honestly, if it were my project, I’d turn this FR down. The next user will probably come and want shell scripts executed … Maybe when an Obsidian command can be executed via some safer mechanism, like a command ID or the like, this could be re-evaluated, or a separate "command plugin" written. Just my 2¢. |
I added this specific functionality because there was already a plugin in the community list that exposed it in the same way, and also the fact that a user has to explicitly enter the name of command to make it work. However, I agree that there is a potential collision issue if the user has a note by the same name as some random Obsidian command, so I think I will do two things:
2 will prevent unintentional collisions with notes. |
Yeah, if you want to keep it, a flag seems a good idea to avoid erroneous file creations and the like. |
3.4.0: Best of both worlds, hee hee. I like best the "X No command found!" when working in the "wrong" language. :-) Hopefully someday will come a better way to access commands. |
Closing this issue as I think the toggle prevents accidental collisions |
Yeah, I saw this when initially putting this functionality in, but these ids aren't revealed to the user at all. I guess I could store the ID when first created, though, instead of the name... |
Pondering … These currently enabled commands could of course be shown using let cmds = dv.array(Object.entries(app.commands.commands))
.sort(v => v[1].id, 'asc');
dv.paragraph(cmds.length + " commands currently enabled.<br><br>");
dv.table(["Command ID", "Name in current locale"],
cmds.map(v => [v[1].id, v[1].name])
); |
This change will be in the next release. |
Just for the record: I wrote a small DataviewJS script to show the current Command Palette: |
How then would we add a Current behaviour (using 3.10.1) is:
|
A commandMarker created in the map block would not update, correct. Not sure that's going to be possible to fix cleanly, since the command IDs are not exposed to the user anywhere except in the console. |
But how else could I create a command marker? "Bulk edit" shows no option for that (if so, a list could be shown and the command id used internally). Keeping a list in your docs and using the internal id seems unreasonable, since plugins can introduce lots of new ones. Hm. Or maybe it could be hidden as a "Marker type" like "Default"? So we could also create command markers on the map using right-clicks. There could be an extra entry like "Command" in there, to make it handle things differently. Then again, we’d lose the marker type capability. Hm again. Personally, I’d be quite happy using the ids in the |
Yeah, it's tricky. A marker can be "converted" into a |
Ah, didn’t know that one! Needs to get into the docs … This actually seems a good way to me—and it works! So why not tell people to set a marker, convert it, and basta? And just "secretly" have the nerdy feature of using the ids for the code block |
It actually was, but accidentally under the Bulk Edit heading. I just fixed it. 😁 |
Also - I'm slowly creating an actual documentation website to host on github. Just need to find the time. |
Yeah, I know how that is—we all probably like better to comment code instead of always running after keeping user docs up-to-date … sigh. |
Just found that it actually puts the command id into the "file to open" field when bulk editing, hee hee. It can’t be changed there, unfortunately. ;-) |
Actually, now that you mention it, it should be able to be changed there (just no command marker <-> note marker toggle). I'm assuming it opens a note list suggestion box on click? I can detect that it's a command marker and open the command suggestion box instead. |
Yep, it tries to open a file then. Interestingly, when setting "command" to "on", it initially even shows the new command id I entered, but when saving, it vanishes. I have to go back in, enable command, and select one. |
I will probably never use a command marker, but this "make a marker and then convert it to a command" might just be the thing for the command marker guys, maybe use a custom map image that shows "functions" and then be able to click around on many commands. :-) |
Yep! Honestly, probably won't ever use it either, but haven't gotten any complaints yet! |
I think I should update my Dataview command list with an id column, just to be sure … |
## 3.11.1 - Fixed issue where opening a map with a marker type that no longer exists would fail to render the map - #38 - Fixed issue where bulk editing markers on maps with command markers was not respecting the Command Marker toggle - #25 - Removed maxZoom constraint from Show All Markers control button
Re #35:
WARNING: This isn’t very stable and will break
Export to PDF
becomesNach PDF exportieren
marker: Location,51,10,Export to PDF
will start the command inEnglish
setting, open the note inDeutsch
setting.I think this simplistic approach is a little too dangerous, don’t you?
Originally posted by @Moonbase59 in #35 (comment)
The text was updated successfully, but these errors were encountered: