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

Change file_name from uuid to highlight_name #8

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lentsd
Copy link

@lentsd lentsd commented Jun 16, 2024

Made file_name readable, so i can read first highlight letters instead of hash

Issue: #7

lentsd added 3 commits June 16, 2024 09:01
forgot to update file_name :)
Add uuid to the end of highlight_name
@lenalebt
Copy link
Owner

Hey, thanks for your contribution!
I'm not opposed to the change in general, but I do also think that this change comes with a few challenges that may not be easy to spot. File systems disallow some characters in file names, and what these are also differs between different OSes. In addition to that, spaces in filenames also can be troublesome in some contexts, not to speak of the different forms of quotations.

I'd suggest to add an allowlist of characters that definitely may be used in filenames (e.g. all alphanumeric characters). I did not want to bother with those special cases, that is why I did not put a quotation in the file name in the first place 🙃 ... What do you think?

@lentsd
Copy link
Author

lentsd commented Jun 17, 2024

Yes, I will update my pull later with a whitelist and logic for escaping bad characters

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