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

Fix: properly escaping pipes in Link.obsidianLink() #2265

Merged
merged 2 commits into from
Mar 17, 2024

Conversation

GottZ
Copy link
Contributor

@GottZ GottZ commented Mar 13, 2024

Closes #2264

please tell me if this is desired behavior or not.

edit:

TS2550: Property 'replaceAll' does not exist on type 'string'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2021' or later.

so.. are you willing to go that step? adding this change is more complicated than replacing it with replace(/\|/g, "\\|")

@blacksmithgu
Copy link
Owner

Obsidian uses a consistent electron version so as long as es2021 is supported in Obsidian it's fine.

Based on checking the developer console this looks fine to me, it seems to be supported.

@GottZ
Copy link
Contributor Author

GottZ commented Mar 15, 2024

ok nice. I'll edit the pull request later this day accordingly :)
thanks!

even though obsidians empty example plugin still specifies es6, the electron runtime of obsidian actually contains es2022 features. Array.prototype.at etc.
@GottZ
Copy link
Contributor Author

GottZ commented Mar 16, 2024

@blacksmithgu the linter is checking for , comma characters at the end of object definitions for good reason.
would you agree that such a big codebase change should be done after the current pull-requests got reviewed or before?
I'll create a separate pull request to make the linter happy anyways, I could also keep it up2date with master, so any linting errors stay visible as file changes.

@GottZ GottZ mentioned this pull request Mar 16, 2024
@blacksmithgu
Copy link
Owner

I'll fix the lints in a followup commit to avoid polluting this one.

@blacksmithgu blacksmithgu merged commit a47ac38 into blacksmithgu:master Mar 17, 2024
2 of 3 checks passed
@GottZ GottZ deleted the patch-2 branch March 17, 2024 08:18
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.

Bug: replace doesn't replaceAll
2 participants