Fix maxLength broken on files with no extension #30
Conversation
|
I like the most the option where only the name without the extension is trimmed. Example: |
filenamify.js
Outdated
| @@ -35,7 +35,9 @@ export default function filenamify(string, options = {}) { | |||
| const allowedLength = typeof options.maxLength === 'number' ? options.maxLength : MAX_FILENAME_LENGTH; | |||
| if (string.length > allowedLength) { | |||
| const extensionIndex = string.lastIndexOf('.'); | |||
| string = string.slice(0, Math.min(allowedLength, extensionIndex)) + string.slice(extensionIndex); | |||
| const filename = extensionIndex > -1 ? string.slice(0, extensionIndex) : string; | |||
There was a problem hiding this comment.
It's more readable to check for === -1 as it cannot be another negative number.
Yes, that would be great, but should be a separate pull request. |
|
I think the max length option is mostly used to prevent abuse and accidental "too long filename" mistakes, so I think just clearly documenting that the length may be larger if the extension is larger than max length, and that it's up to the user to check the length of the returned string if they allow arbitrary file extensions. |
All right, I'll submit it after this is merged, as it affects the same lines of code. |
Currently, files with no extension are not accounted for when satisfying
maxLength, which results in them not being trimmed at all (accident oflastIndexOf()returning-1).This PR fixes it.
BUT! There is another thing which I personally consider an issue. Currently,
maxLengthtrims only the filename part to the desired length, and then attaches the extension. I haven't changed this behavior in this PR, but I personally don't consider it right. If I want a filename to be 100 chars long, then it should be 100 chars long including the extension, not 100 chars + extension length.Should I fix this too? And if so, what would be the behavior if it can't be satisfied due to the extension itself being longer than the
maxLengthrequirement?The options are:
maxLength, breaks something else potentially.maxLengthcan't be satisfied without breaking something, but this will bite people in the ass, as this is one of those things that tends to happen only in production.