node-sanitize-filename icon indicating copy to clipboard operation
node-sanitize-filename copied to clipboard

Add options for not doing Windows-specific rules, not truncating

Open flotwig opened this issue 6 years ago • 5 comments

I wanted to use this in the context of a web application which saves files on a GitHub repo (flotwig/markdown-notebooks) so I didn't need or want the parts that applied only to Windows filenames. So, I added an option to disable the last 2 regexes that are applied that are Windows-specific.

I also added an option to disable truncation, just for the sake of having some more options. :)

I added unit tests for the new functionality along with a switch for test.js to just run the unit tests. All tests pass, and this doesn't change the behavior the exposed API or the internal sanitize API for existing users.

flotwig avatar Jan 19 '19 04:01 flotwig

Hi @flotwig, thanks for picking this up. I think a nicer way to add this functionality is adding a target option (perhaps an object with unix and windows properties, as boolean flags), see the discussion in #11 and #9 for the motivation. Particularly, the naming of that option needs to reflect that one is sanitizing for a certain environment, and not necessarily the environment they're on. target is just one suggestion :)

joelmukuthu avatar Jan 19 '19 12:01 joelmukuthu

That sounds good. So I make sure I get this right, the unix flag would basically only target the third regex for .. and ., since the other two are multi-platform?

flotwig avatar Jan 19 '19 15:01 flotwig

Hmm, I'm actually not sure that . and .. are valid on windows. @parshap, you probably know more on this? :)

joelmukuthu avatar Jan 20 '19 00:01 joelmukuthu

Also, @parshap, is the 255 max-length a thing on all platforms?

joelmukuthu avatar Jan 20 '19 00:01 joelmukuthu

~~I'm using macOS 10.13.6 and my volume filesystem is APFS, the longest filename length looks like 241, not 255.~~

I was wrong, it is 255 but includes the filename extension (means XXX.md the .md parts will be counted in).

jackycute avatar Feb 24 '19 06:02 jackycute