Add leading underscore option #43

dgaussr · 2020-01-27T20:16:17Z

[Fixes #39]
I created a new leading underscore parameter for the function as false at default. But, when it's true, it changes the current regexp to accept the leading underscore:

Example:

slugify("_foo bar", { leadingUnderscore: true })

Output: _foo-bar

sindresorhus · 2020-01-29T14:49:58Z

You need to write in the PR description what issue the PR fixes in the form: Fixes #123.

sindresorhus · 2020-01-29T14:45:12Z

test.js

+
+test('leading underscore', t => {
+	t.is(slugify('_foo bar', {leadingUnderscore: true}), '_foo-bar');
+	t.is(slugify('_foo_bar', {leadingUnderscore: true}), '_foo-bar');


Can you add a test for __foo__bar? (Multiple leading underscores).

sindresorhus · 2020-01-29T14:45:36Z

readme.md

@@ -132,6 +132,23 @@ slugify('foo@unicorn', {
 //=> 'foo-at-unicorn'
 ```

+##### customReplacements


sindresorhus · 2020-01-29T14:46:10Z

readme.md

+Type: `boolean`\
+Default: `false`
+
+Allow leading underscore as an escape `_foo_bar` → `_foo-bar`.


It's not really an "escape". I think it would be more correct to say that it preserves leading underscore.

hmm understood gonna change it today!

sindresorhus · 2020-01-29T14:47:12Z

readme.md

+slugify('_foo_bar');
+//=> 'foo-bar'
+
+slugify('_foo_bar', {leadingUnderscore: true});


I know it's verbose, but I think this would be clearer:

Suggested change

slugify('_foo_bar', {leadingUnderscore: true});

slugify('_foo_bar', {preserveLeadingUnderscore: true});

index.js

sindresorhus · 2020-01-29T14:50:47Z

You also need to document it in index.d.ts.

sindresorhus · 2020-02-01T05:49:02Z

test.js

-	t.is(slugify('_foo_bar', {leadingUnderscore: true}), '_foo-bar');
+	t.is(slugify('_foo bar', {preserveLeadingUnderscore: true}), '_foo-bar');
+	t.is(slugify('_foo_bar', {preserveLeadingUnderscore: true}), '_foo-bar');
+	t.is(slugify('__foo__bar', {preserveLeadingUnderscore: true}), '_-foo-bar');


This should result in:

Suggested change

t.is(slugify('__foo__bar', {preserveLeadingUnderscore: true}), '_-foo-bar');

t.is(slugify('__foo__bar', {preserveLeadingUnderscore: true}), '_foo-bar');

Would also be good to add a test for slugify('____-___foo__bar', {preserveLeadingUnderscore: true}), which should also result in _foo-bar.

I was testin and after the

string = string.replace(patternSlug, separator);

in all these examples it became _\-foo\-bar.

So, I thought in just slicing the separator (\-) after it. Something like:

if(string.slice(1,2) == separator){ string = string.slice(0, 1) + string.slice(2, string.length); }

it works, but what do you think?

I think you're unnecessarily complicating the implementation. You could just do:

const shouldPrependUnderscore = options.preserveLeadingUnderscore && string.startsWith('_');

at the start and then prepend a underscore at the end if shouldPrependUnderscore is true.

I actually thought that way firstly, but i thought would be better to just modify the current regex and got confused afterall

just committed the changes, sindre.

sindresorhus · 2020-02-01T05:50:48Z

readme.md


 Type: `boolean`\
 Default: `false`

-Allow leading underscore as an escape `_foo_bar` → `_foo-bar`.
+It preserves leading underscore: `_foo_bar` → `_foo-bar`.


I think this could be better written as:

Suggested change

It preserves leading underscore: `_foo_bar` → `_foo-bar`.

If your string starts with an underscore, it will be preserved in the slugified string.

sindresorhus · 2020-02-01T05:51:39Z

You need to write in the PR description what issue the PR fixes in the form: Fixes #123.

The description, not title.

sindresorhus · 2020-02-05T05:34:17Z

readme.md

+Type: `boolean`\
+Default: `false`
+
+If your string starts with an underscore, it will be preserved in the slugified string.


You need to keep the readme in sync with the index.d.ts docs.

I think it would also be useful to include a use-case for this option, as mentioned by the original issue.

I have just added an example as a hidden_filename to give this idea

Sorry, I should have been clearer, by use-case, I meant a written use-case, not a code example. I don't think _hidden-filename clarifies much.

Right. and if I just update the description to:
"If your string starts with an underscore, it will be preserved in the slugified string. Used in filenames to represent a hidden path."

I still don't think that's very clear.

You could almost just copy-paste the comment from the original issue:

Sometimes leading underscores are intentional, for example, filenames representing hidden paths on a website.

ok, perfect! done that!

sindresorhus · 2020-02-05T05:36:11Z

index.js

@@ -64,10 +65,16 @@ const slugify = (string, options) => {
 		patternSlug = /[^a-z\d]+/g;
 	}

+	const shouldPrependUnderscore = options.preserveLeadingUnderscore && string.startsWith('_');


This check should be done before any string transformations are done.

Indeed! Done.

sindresorhus · 2020-02-11T21:52:40Z

Thanks :)

Add leading underscore option

64d6bbc

sindresorhus requested changes Jan 29, 2020

View reviewed changes

sindresorhus review changes

0c20cd6

dgaussr changed the title ~~Add leading underscore option~~ Add leading underscore option [Fixes ##39] Feb 1, 2020

dgaussr changed the title ~~Add leading underscore option [Fixes ##39]~~ Add leading underscore option [Fixes #39] Feb 1, 2020

sindresorhus reviewed Feb 1, 2020

View reviewed changes

dgaussr changed the title ~~Add leading underscore option [Fixes #39]~~ Add leading underscore option. Feb 3, 2020

fixes in the code and in the readme file

9348c29

sindresorhus reviewed Feb 5, 2020

View reviewed changes

Update index.js

ee0cf26

sindresorhus reviewed Feb 5, 2020

View reviewed changes

sindresorhus changed the title ~~Add leading underscore option.~~ Add leading underscore option Feb 5, 2020

dgaussr and others added 6 commits February 5, 2020 19:43

fix in the order of the condition and in readme

ea9be90

fix in the order of the condition and in readme

e071769

fix in the order of the condition and in readme

35b8da5

readme update. include use-case

ed03eeb

Update index.d.ts

85209f8

Update readme.md

781c37e

sindresorhus merged commit 9a78d8f into sindresorhus:master Feb 11, 2020

vhpoet mentioned this pull request Apr 21, 2021

Add preserveTrailingDash option #57

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add leading underscore option #43

Add leading underscore option #43

dgaussr commented Jan 27, 2020 •

edited

Loading

sindresorhus commented Jan 29, 2020

sindresorhus Jan 29, 2020

dgaussr Jan 31, 2020

sindresorhus Jan 29, 2020

sindresorhus Jan 29, 2020

dgaussr Jan 31, 2020

sindresorhus Jan 29, 2020

sindresorhus commented Jan 29, 2020

sindresorhus Feb 1, 2020

dgaussr Feb 1, 2020

sindresorhus Feb 3, 2020 •

edited

Loading

dgaussr Feb 3, 2020

dgaussr Feb 3, 2020

sindresorhus Feb 1, 2020 •

edited

Loading

sindresorhus commented Feb 1, 2020

sindresorhus Feb 5, 2020

sindresorhus Feb 5, 2020

dgaussr Feb 5, 2020

sindresorhus Feb 6, 2020

dgaussr Feb 6, 2020

sindresorhus Feb 6, 2020

dgaussr Feb 8, 2020

sindresorhus Feb 5, 2020

dgaussr Feb 5, 2020

sindresorhus commented Feb 11, 2020

	slugify('_foo_bar', {leadingUnderscore: true});
	slugify('_foo_bar', {preserveLeadingUnderscore: true});

	t.is(slugify('__foo__bar', {preserveLeadingUnderscore: true}), '_-foo-bar');
	t.is(slugify('__foo__bar', {preserveLeadingUnderscore: true}), '_foo-bar');

	It preserves leading underscore: `_foo_bar` → `_foo-bar`.
	If your string starts with an underscore, it will be preserved in the slugified string.

Add leading underscore option #43

Add leading underscore option #43

Conversation

dgaussr commented Jan 27, 2020 • edited Loading

sindresorhus commented Jan 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus commented Jan 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus Feb 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

sindresorhus commented Feb 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus commented Feb 11, 2020

dgaussr commented Jan 27, 2020 •

edited

Loading

sindresorhus Feb 3, 2020 •

edited

Loading

sindresorhus Feb 1, 2020 •

edited

Loading