mozilla / scanjs

[DEPRECATED] Static analysis tool for javascript code.
Other
429 stars 39 forks source link

Replace dollar placeholder #148

Closed mozfreddyb closed 10 years ago

mozfreddyb commented 10 years ago

If we ever want to scan something like jQuery, we might not want $ to have a meaning in ScanJS. I replaced it with §, mainly because they look so similar and are next to each other on my keyboard. (I guess I could have picked something less common, like µ ;))

Are you OK with this, @pauljt?

mozfreddyb commented 10 years ago

Ha... § isn't even a valid identifier.

We can still pick µ. I will abort the pull request, but let's keep this open to discuss which character to pick :P

@pauljt?

mozfreddyb commented 10 years ago

cf. http://mathiasbynens.be/notes/javascript-identifiers

mozfreddyb commented 10 years ago

I rebased with µ and added a convenient µ-Button to the experiment field for people without the key on their keyboard.

pauljt commented 10 years ago

Honestly I would rather keep it as something ascii. Typing mu is the pain in the arse if you are editing in a text editor, and might lead to issues. Doesn't have to be one character - could be $$ or seomthing weirder.

Please let me review before doing this though as there are some tricky parts that will need to change.

On 26 May 2014, at 6:13 pm, Frederik notifications@github.com wrote:

I rebased with µ and added a convenient µ-Button to the experiment field for people without the key on their keyboard.

— Reply to this email directly or view it on GitHub.

mozfreddyb commented 10 years ago

$$ is commonly used for querySelectorAll (where $ is querySelector).

For me µ is no the keyboard ;) just like ß. It should be on your keyboard with alt+option-m or so. I added a button in the experiment tab that adds the special character to the input field. I don't think there are other viable alternatives..

pauljt commented 10 years ago

The proper alternative is to add escaping. But if you really want, I could live something like $_any.foo

Please don’t add weird letters that can’t be enter via the keyboard. A soft key is not a good answer here.

On 27 May 2014, at 8:06 am, Frederik notifications@github.com wrote:

$$ is commonly used for querySelectorAll (where $ is querySelector).

For me µ is no the keyboard ;) just like ß. It should be on your keyboard with alt+option-m or so. I added a button in the experiment tab that adds the special character to the input field. I don't think there are other viable alternatives..

— Reply to this email directly or view it on GitHub.

mozfreddyb commented 10 years ago

shrugs $_?

pauljt commented 10 years ago

So upon looking at @codelion 's PRs, it dawns on me that we actually need several meta placeholder thingies. I propose:

$_any : always matches $_contains(NodeType): walks down the tree for the specified node, and returns true if any of the nodes match the specified type.

Then you could do:

$_any.innerHTML = $_contains("Identifier")

(matches innerHTML property being assigned, but only when there is at least one identifier of on the right hand side)

pauljt commented 10 years ago

What do you think?

pauljt commented 10 years ago

And actually i would probably rename matchArgs to $_matchArgs too

mozfreddyb commented 10 years ago

this makes sense to me :)

pauljt commented 10 years ago

Made the change to use $_any.

pauljt commented 10 years ago

Automated tests pass, manual testing still seems to work, so merging.