stripTags strips things that aren't tags

[paulfalgout]

No description provided.

[stoeffel]

Thanks for pointing us to this issue. Do you want to fix this bug?

[paulfalgout]

@stoeffel Regex isn't really my thing, so this heavily borrows from
https://github.com/kvz/phpjs/blob/master/functions/strings/strip_tags.js

[paulfalgout]

Funny enough FWIW #266 is also heavily borrowing from the same source and would also solve this problem, but adds the ability to allow certain tags (why the author renamed allow to ignore, I'm not sure)

[JSteunou]

@stoeffel any chance to get this one merged? This issue is kind of severe :/

[stoeffel]

will review it tomorrow. sorry for the delay. we will be faster with our
new build chain.
On Feb 24, 2016 11:54, "Jérôme Steunou" [email protected] wrote:

@stoeffel https://github.com/stoeffel any chance to get this one
merged? This issue is kind of severe :/

—
Reply to this email directly or view it on GitHub
#469 (comment)
.

[JSteunou]

Nice! Thank you for your work.

[stoeffel]

I tried this with some additional tests and the last one fails.

  equal(stripTags('<h1 id="foo" data-foo="bar">hello world</body></h1>'), 'hello world');
  equal(stripTags('<web-component>hello world</web-component>'), 'hello world');
  equal(stripTags('<ReactComponent.Title>hello world</ReactComponent.Title>'), 'hello world');
  equal(stripTags('I have <I want, but that is > nothing'), 'I have <I want, but that is > nothing');

  78 passing (32ms)
  1 failing

  1)  #stripTags:

      AssertionError: 'I have  nothing' == 'I have <I want, but that is > nothing'
      + expected - actual

-       I have  nothing
+      I have <I want, but that is > nothing

      at Context.<anonymous> (tests/stripTags.js:13:3)

[JSteunou]

I'm afraid this is the closest strip tags can be done with regex. True strip tags can only be done with parser.

[stoeffel]

Yes, you are right. It only looks strange 😸

[stoeffel]

Could you include a test for comments and maybe some tests like I described above? Otherwise this looks good. Thanks for the PR.

[paulfalgout]

a test for comments

Can you clarify this?

[stoeffel]

I mean a test for <!-- a html comment --->.
On Feb 24, 2016 16:54, "Paul Falgout" [email protected] wrote:

a test for comments

Can you clarify this?

—
Reply to this email directly or view it on GitHub
#469 (comment)
.

[paulfalgout]

Ok good deal. I'll see about getting to this shortly

[stoeffel]

nice. thanks
On Feb 24, 2016 17:01, "Paul Falgout" [email protected] wrote:

Ok good deal. I'll see about getting to this shortly

—
Reply to this email directly or view it on GitHub
#469 (comment)
.

[stoeffel]

The more I think about this, the more I think we should consider deprecating striptags (like we did with sprintf). I think the problem stripTags is trying to solve is too big for _.str and a module like https://www.npmjs.com/package/striptags does a much better job. What are your thoughts?

[paulfalgout]

I'm good with that. My biggest beef with this function is that I assumed a little too much that it would work, but it was aggressively stripping things that were valid inputs from users. This implementation is better but I've gone in another direction myself as I can't have either a false sense of security or false positives. Probably for something as sensitive as this functionality it should come from a library that is very focused on the task.

[JSteunou]

Ho did not know about this lib. Good idea. About sprintf & striptags, why deprecated it? I mean, I consider underscore.string like the Swiss army knife when it comes to deal with Strings and I like having all those features in one place, even if that means some features are just symbolic links to another library. I trust underscore.string enough to let it choose for me the best library for the job under the hood.

[stoeffel]

About sprintf & striptags, why deprecated it?

Having dependencies means more maintenance. We would have to deal with issues for 3rd party libraries that aren't under _.str control. We would have to document the functionality of these modules, which is overhead and can lead to misleading documentation.
With deprecating this functions and temporarily linking to the other module, we can make users aware of that module, which solves the problem much better.

[JSteunou]

Make sense. Thank you.

[stoeffel]

@epeli do you agree with deprecating stripTags?