nothingislost / obsidian-dynamic-highlights

An experimental Obsidian plugin that highlights all occurrences of the word under the cursor
MIT License
132 stars 7 forks source link

Bug:The new version of the plugin will cause another plugin obsidian-shellcommands to fail #25

Closed tyf2018 closed 2 years ago

tyf2018 commented 2 years ago

The new version of the plugin will cause another plugin obsidian-shellcommands to fail. The old version 0.1.2 is normal and will not conflict.

Can this compatibility issue be resolved? Thanks!

obsidian-shellcommands: https://github.com/Taitava/obsidian-shellcommands

Best Regards.

nothingislost commented 2 years ago

I don't use that plugin. Can you provide some more details around how the plugin fails? If there are any errors in the dev tools console, that would be helpful.

tyf2018 commented 2 years ago

Hi, @nothingislost

Here is the console log. obsidian.md-1642819080523.log

image

image

Mara-Li commented 2 years ago

Oh, so, I'm not the only one. If at last I check the issue here before I make a issue on shell commands...

So, here is the discussion in Shell commands : https://github.com/Taitava/obsidian-shellcommands/discussions/137 There is no message from Dynamic, but on Shell :

image

Taitava commented 2 years ago

Hi, I'm the maintainer of the mentioned plugin. I'm currently in a rush and don't have time to read through this issue thoroughly. I just wanted to say that if you need any help from me inspecting this issue (or if there's anything to fix in my plugin), I'm ready to help. :slightly_smiling_face:

Taitava commented 2 years ago

@tyf2018 In the title you mention The new version of the plugin will cause.... Does new version mean 0.1.3? This can help me to look into this. If this is the case, then I can compare the changes between 0.1.2 and 0.1.3.

tyf2018 commented 2 years ago

@tyf2018 In the title you mention The new version of the plugin will cause.... Does new version mean 0.1.3?

Thanks for taking the time to fix this, the new version refers to 0.1.6.

In fact, I just also tried the 0.2.1 version of dynamic-highlights, which also caused the error.

chrisgrieser commented 2 years ago

I can also confirm that disabling dynamic highlights allows me to use the settings tab of Shell Commands again.

chrisgrieser commented 2 years ago

also, the Dynamic Highlights plugin seems to dislike shell commands so much, when I have entered new settings in the shell commands by disabling dynamic highlights, and then enable dynamic highlights again, it completely shuts me off from the shell commands:

Screenshot 2022-02-13 12 07 30

Also, shell commands that include variables are also blocked as long as Dynamic highlights is enabled.

nothingislost commented 2 years ago

@Taitava this appears to be due to the fact that I'm patching RegExp Match to include capture group indices (https://github.com/tc39/proposal-regexp-match-indices) This is an additive patch and shouldn't change the default behavior of RegExp Match but it appears the fact that the match data structure now has additional fields is throwing off some of your logic:

Without the patch image

With the patch image

Notice the new indices key. That appears to be breaking your logic here: https://github.com/Taitava/obsidian-shellcommands/blob/2b05ffd955ebf57288d9cb03551267c564ff42b5/src/variables/parseShellCommandVariables.ts#L39 because it's iterating over the indices key and parameter_name is undefined image

I'll look into what I can do to limit the scope of this patch but the ECMAScript proposal for this RegExp feature has been accepted so it would probably be good to handle it within Shell Commands if possible,

nothingislost commented 2 years ago

v0.2.2 of Dynamic Highlights should fix this incompatibility. The RegExp shim has been changed to not affect all instances of RegExp so it should no longer affect other plugins.

chrisgrieser commented 2 years ago

okay, it got a bit better but we are not there yet. I may open the shell commands and enter variables there, but only partially. Not sure which plugin exactly causes the problem, therefore posting it here. Like when I try to enter a variable, the last letter of the plugin does not get accepted/saved? ლ(ಠ_ಠლ)╯

image

nothingislost commented 2 years ago

Did you restart Obsidian after upgrading Dynamic Highlights? I don't see this behavior.

chrisgrieser commented 2 years ago

oh right, I haven't.

After restart, everything works now. Thanks!

Taitava commented 2 years ago

v0.2.2 of Dynamic Highlights should fix this incompatibility. The RegExp shim has been changed to not affect all instances of RegExp so it should no longer affect other plugins.

Thank you a lot! 😎 Nice that you found a solution! 🙂 It might have been hard for me to spot this regex issue.

Notice the new indices key. That appears to be breaking your logic here: https://github.com/Taitava/obsidian-shellcommands/blob/2b05ffd955ebf57288d9cb03551267c564ff42b5/src/variables/parseShellCommandVariables.ts#L39 because it's iterating over the indices key and parameter_name is undefined image

I'll look into what I can do to limit the scope of this patch but the ECMAScript proposal for this RegExp feature has been accepted so it would probably be good to handle it within Shell Commands if possible,

I'll definitely need to change that part of the code. I created a new issue for SC for fixing it: https://github.com/Taitava/obsidian-shellcommands/issues/152

Taitava commented 2 years ago

Just for the record, I've fixed the bad logic in my plugin now. The fix is just not released yet, but will be in SC 0.11.0. Thank you to everybody for the co-operation! 😎