-
Notifications
You must be signed in to change notification settings - Fork 48
Clean note filename. #196
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
Clean note filename. #196
Conversation
4951aec
to
614fa79
Compare
What about german umlaute? They would be replaced by an underscore too.
|
What about non-latin letters like cyrillic? |
@slashrsm I wouldn’t necessarily go so far just to fix #167. The one specific problem we really have is the one of leading Because as the others mentioned too – why would you want to replace umlaute and other characters, dashes etc? Also, the underscore should never ever be in a filename. It is an ugly character only meant for underlining back in the typewriter days. Doesn’t look nice but only technical in filenames. |
Updated based on what @jancborchardt suggested. Thank you for the input. |
@@ -112,6 +112,9 @@ public function update ($id, $content, $userId){ | |||
|
|||
// prevent directory traversal | |||
$title = str_replace(array('/', '\\'), '', $title); | |||
// remove hash and space characters from the beginning of the filename | |||
// in case of markdown | |||
$title = ltrim($title, ' #'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn’t it be #(space)
instead of (space)#
? And also ideally trim just the plain #
as well in case someone doesn’t put a space behind the hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ltrim() will trim any character in the character list. Order is not important. "# foo", " #foo", "#foo", " foo", " foo" will all end up as "foo" after this gets executed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah cool, thanks for the info! :)
See my inline comment. :) Also @Henni @BernhardPosselt please check, @slashrsm when you’re done, can you please squash your commits into one so we have a clean history? Thanks! :) |
53b2bc2
to
c04d550
Compare
@jancborchardt Thank you for your feedback. Commits squashed. |
I can’t really review the code but it looks good. @suhr @calis2002 @Henni @LukasReschke @BernhardPosselt can you check it out? |
Any thoughts on this? |
Can you please review this @MorrisJobke @stefan-niedermann @Henni @suhr @calis2002 @LukasReschke |
Thank you. Works fine 🚀 |
Removes unwanted characters from the note's filename.
Fixes #167