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

Remove trailing and leading single/double quotes from path #1640

Merged
merged 4 commits into from
Nov 5, 2021

Conversation

disberd
Copy link
Contributor

@disberd disberd commented Nov 5, 2021

Fixes #1639

Should not break existing valid names containing quotes as it only remove the trailing and leading character if there is a quote after the notebook extension

Fixes fonsp#1639

Should not break existing valid names containing quotes as it only remove the trailing and leading character if there is a quote after the notebook extension
@dralletje
Copy link
Collaborator

A bit niche, but sure..

But can you make it very strictly only work when both the first and last character are double quotes?
And also add a comment referring to the issue you made.

@disberd
Copy link
Contributor Author

disberd commented Nov 5, 2021

Done, I also kept single quote as option as I suppose there are cases where the path are pasted surround by them but I can remove them if you think they are breaking something.

@dralletje
Copy link
Collaborator

It's not about breaking stuff, but about keeping the code we insert for an exception really simple and easy to follow (e.g. path_or_url[0] === '"' && last_char === '"' vs whatever there is now). I know the difference seems silly, but having an special handling like this really needn't be something we do often :P

Also can you use .slice instead of .substring? That way you can write .slice(1, -1) 😎

Also also, can you run prettier? Now if I open the file I get a diff instantly because of the formatting

@disberd
Copy link
Contributor Author

disberd commented Nov 5, 2021

Makes sense, I have the tendency of try to take into account too many un-necessary corner cases...

Thanks for the slice suggestion, my Javascript expertise always relies on google what I want to achieve :D.

I removed the definition of last_char since probably is clear enough with indexing the string directly.
Let me know if that is OK.

I didn't have any prettier as I started this edit directly on the web github interface but this time I pulled this branch on VSCode to prettify.

@dralletje
Copy link
Collaborator

Thanks a lot! Sorry for making you go through 2 edits :P

@dralletje dralletje merged commit b236b8e into fonsp:main Nov 5, 2021
@disberd disberd deleted the patch-5 branch November 5, 2021 17:14
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.

FilePicker does not support paths surrounded by double/single quote
2 participants