2

As far as I can tell, my RegEx is correct and the replace method should be doing it's thing. Looking at the answers here and here, it seems I am using the same, or a very very similar, RegEx pattern, and they are accepted answers.

So I'm thinking perhaps I'm doing something wrong with the .replace method?

Ideally, the RegEx should match anything not contained in the character class, meaning anything that is not: a-z, 0-9, '-', '.', and '_'. I read that there is no need to escape special characters within character classes (except '[', ']', and '-') so I have only escaped the hyphen.

Any help is much appreciated. To clarify; passing the below function this string:

'_;<!--Invalid--!"s"' null' should return ____--invalid--____null.

function nameSafe(name) {
  var rgxIllegalChars = /^[^a-z0-9\-._]+$/;
  name = name.toLowerCase();
  return name.replace(rgxIllegalChars, '_');
}

Edit: Solved, see accepted answer for information. Finished function:

function nameSafe(name) {
  var rgxIllegalChars = /[^a-z0-9\-._]+/gi;
  return name.replace(rgxIllegalChars, '_');
}

I added some debugging output to my function to try and catch the before and after, and somewhat foolishly forgot that .replace doesn't actually replace the original string. So I was doing this:

console.log(name);
name.replace(rgxIllegalChars, '_');
console.log(name);
return name;

My apologies - if you're also having trouble, never forget to check your absolute basics (like I did) - does the method your using return a new object, or replace the existing one?

Community
  • 1
  • 1
GrayedFox
  • 2,350
  • 26
  • 44

2 Answers2

2

You just need to remove anchors:

function nameSafe(name) {
  var rgxIllegalChars = /[^a-z0-9\-._]+/;
  name = name.toLowerCase();
  return name.replace(rgxIllegalChars, '_');
}

Edit:

var name = "'_;<!--Invalid--!\"s\"' null'";
var rgxIllegalChars = /[^a-z0-9\-._]+/g;
name = name.toLowerCase();
alert(name.replace(rgxIllegalChars, '_'));

Note this line var rgxIllegalChars = /[^a-z0-9\-._]+/g;

Thomas Ayoub
  • 29,063
  • 15
  • 95
  • 142
  • Thanks for the speedy answer! I tried that, but unfortunately it doesn't work. I also logged the result before and after, to ensure it was actually being called - input and output is the same :( – GrayedFox Nov 12 '15 at 14:18
  • Your code 100% works in my browser, running through dev console. For some weird reason, I'm still not getting the expected result in my output... maybe this thing runs deep :( – GrayedFox Nov 12 '15 at 14:43
  • Updated OP to shed light on the issue. Embarrassing... removing anchors did it, really, I just got schloppy. Thanks for your time – GrayedFox Nov 12 '15 at 15:08
2

In your regular expression ^ means assert position at start of the string and $ means assert position at end of the string once you remove these two it will give you your expected result.

I have created a sample on Regex101.com for you https://regex101.com/r/xF7vA8/3, click on Substitution button to see the result.

enter image description here

ndd
  • 3,051
  • 4
  • 25
  • 40
  • This certainly seems to work at Regex101.com - and I noticed you added the g tag (which I should also) - but my results, strangely, are still the same before and after the function. I'm worried now it's something to do with async – GrayedFox Nov 12 '15 at 14:24
  • Can you post complete code? May be we can help you better after having a look at complete problem. – ndd Nov 12 '15 at 14:26
  • 1
    while you are at it, add the `i` flag and remove the `toLowerCase()` stuff – mkoryak Nov 12 '15 at 14:28
  • Is the + important? I thought it was needed (to match any single char within the character class any number of times) but perhaps the -g flag negates the need for this in JS? – GrayedFox Nov 12 '15 at 14:51
  • 1
    I think you don't need + as it will change meaning of your expression have a look at https://regex101.com/r/xF7vA8/4 and pay attention to 'Substitution' result by adding and removing +, when you add it, it does explain saying `Quantifier: + Between one and unlimited times, as many times as possible, giving back as needed [greedy]` – ndd Nov 12 '15 at 14:54
  • I did see that, and noticed it still matches what I need it to. Interestingly, adding a + will replace multiple instances of illegal chars with a single '_', whereas leaving it out ensures there is one '_' per matched illegal char in the string. Not sure about the performance benefits! – GrayedFox Nov 12 '15 at 15:01