Remove $ from iskeyword in PHP files

[chrisyue]

No description provided.

[goatslacker]

I'm confused, which file is this?

[amadeus]

I think what he means is that when a PHP file loads this JS, to not have $ be iskeyword? This might or might not be possible, we'd have to look at the standard PHP file

[zeroasterisk]

I think you should replace this line:

https://github.com/pangloss/vim-javascript/blob/master/syntax/javascript.vim#L26

With this

autocmd FileType javascript setlocal iskeyword+=$

So that it only sets it on javascript files...

[ghost]

Yes please fix, this is very annoying for PHP users that use this plugin :(

[zeroasterisk]

as a temp fix you can do the following:

https://github.com/zeroasterisk/home/blob/master/.vimrc#L375

autocmd FileType php setlocal iskeyword-=$
autocmd FileType javascript setlocal iskeyword+=$

[amadeus]

So, this only represents my opinion on the matter (that of a single maintainer), not necessarily representative of all the other maintainer on this project.

If we look at the problem as it exists, it's a differing implementation between PHP and Javascript. This is a FileType plugin dedicated to Javascript. Javascript knows nothing about PHP, in fact it's the other way around. PHP is the superset that incorporates Javascript into its templates.

Furthermore, jQuery is a very popular library that depends heavily on the $ as a keyword. I don't think this plugin should do anything to fix PHP. In fact I believe the reverse. It should be PHP that fixes the problem since it's the language that is incorporating the Javascript.

So, the TL;DR from my perspective is I vote for a won't fix, other than perhaps a note in the README for PHP developers who use this plugin.

Just a note for you guys on your autocmd fix. The way you have it now, every time you source your vimrc, (or if this was added to a syntax file, every time you source that syntax file) it's going to add ANOTHER autocmd. What you should do is something more akin to this:

augroup phpjsfix
    autocmd!
    autocmd FileType javascript setlocal iskeyword+=$
    autocmd FileType php setlocal iskeyword-=$
augroup END

This will ensure that every time you source your vimrc or a respective syntax file with that snippet included, it will first clear those autocmds before adding them.

[davidchambers]

It should be PHP that fixes the problem since it's the language that is incorporating the Javascript.

I find this argument compelling. It's up to the PHP highlighter to behave appropriately in the various contexts which can exist in a .php file (PHP, HTML, CSS, and JavaScript).

[zeroasterisk]

Hold on - i fully agree that "It's up to the PHP highlighter to behave appropriately"

That makes a whole lot of sense.

But what I take issue with is that this plugin is making a globally scoped change in vim that is unexpected... namely adding $ to iskeyword.

You're argument could very easily be modified to "It's up to the javascript plugin to only affect javascript file when setting defaults"

PHP is just one example where changing the behaviour of $ kinda breaks things... there are other syntaxes where it would also be bad for. It just seems very heavy-handed for this javascript plugin to make globally scoped changes that affect every other syntax too.

---

oh, and @amadeus thanks for the autocmd help!

[qstrahl]

The problem is that Vim does not provide very good support for having nested syntaxes. In JavaScript, $ is a keyword character. The only way to reflect this in Vim -- the only way for JavaScript syntax to behave appropriately -- is to add $ to iskeyword. It's not feasible for JavaScript syntax to support every other syntax in the world that might want to embed it; it's the responsibility of other syntaxes embedding JavaScript to make sure they behave as their users expect. Our syntax plugin behaves the way our users expect.

[zeroasterisk]

Well I guess we agree to disagree.

If vim does default $ to a keyword and I install a JavaScript plugin, I
would not then expect vim to treat $ differently on non-JavaScript content.
Say in HTML or XML or Perl or whatever. My expectations about keywords,
especially on non-JavaScript content, would be that noting has changed by
me installing a JavaScript plugin.

I'll stop beating this dead horse, as I have a viable workaround. But I
suspect this will frustrate many others as well.

Thanks,
-alan

[ghost]

I'm with 0*, IMHO i don't get why making $ to be part of the iskeyword option in ANY file (JS or not JS) is not considered a bug.

[qstrahl]

If vim does default $ to a keyword and I install a JavaScript plugin, I would not then expect vim to treat $ differently on non-JavaScript content.

It doesn't; it treats $ differently whenever you tell vim that the syntax is JavaScript, as your PHP syntax likely does. It's the fault of the PHP syntax for not setting iskeyword explicitly after loading other syntax files that might interfere. Even then, your javascript syntax is going to behave incorrectly, because $ will not be treated like a keyword where it should. It's just a question of which way you want it to be wrong.

What is more reasonable, to expect us to account for a potentially infinite number of other syntax plugins that want to embed JavaScript, or for other syntax plugins to pay attention to the scripts they embed?

I'm with 0*, IMHO i don't get why making $ to be part of the iskeyword option in ANY file (JS or not JS) is not considered a bug.

In JavaScript syntax, $ is a keyword character. Thus, when you tell vim to load JavaScript syntax, $ should be treated like a keyword, unless the situation is exceptional -- for example, if the JavaScript is merely embedded in another language for which $ is not a keyword character. In this case, the other language plugin has decided to include this plugin; we have not decided to be included. The onus is on them to ensure that their shortcut doesn't screw up behaviour.

[davidchambers]

Related: #196

[amadeus]

There seems to be a parallel discussion also going on regarding the validity of setting iskeyword.

For the record, we don't set iskeyword, we setlocal iskeyword. This means it's on a per buffer instance, not a global, per se.

Furthermore, I did a quick grep throughout the default vim runtime syntax files, and there are around 120 instances of setlocal iskeyword being used.

In summary, I believe the way we are doing it now is the standard way the community does it. It's not the job of this plugin to fix another syntax file which embeds our own. And onus should be on the PHP maintainer to provide a fix for other languages that they may be embedding.

I, (and I assume we) would be happy to accept a pull request that puts a note in the README regarding this issue, assuming the aforementioned fix works (I haven't done anything with PHP in years, so I've never come across this issue.)

[qstrahl]

I would like to extend the suggestion for a fix to the authors of the syntax plugins in question; those of you experiencing bad behaviour, inform the maintainers of the root-level syntax plugin (probably php) sourcing JavaScript that they need to setlocal iskeyword after sourcing embedded languages.

I'd be happy to make issues / pull requests against the other syntax plugins if somebody would like to point me at their repositories.

[qstrahl]

A cursory glance at the comments in my $VIMRUNTIME/syntax/php.vim tells me this is the upstream. Could I please get @chrisyue, @zeroasterisk, and/or @sp-gonzalo-serrano to confirm that theirs comes from the same place?

[ghost]

Hi,

Sorry for the delay, forgot this till i re-installed everything and got the same problem again :P

I'm using spf13 vim distributuion but instead of the default PHP stuff from https://github.com/spf13/PIV i'm using https://github.com/StanAngeloff/php.vim. I'm not an expert but as you say i think it embeds HTML & JS stuff so the JS syntax file will be loaded and the isk+=$ will be executed, so meh for me.

Maybe @StanAngeloff has more hints about this stuff?

[StanAngeloff]

@sp-gonzalo-serrano late to the party, see the referenced commit in the php.vim repository. The iskeyword setting is saved and restored so the $ should not sneak in any more, unless you've configured it as such.

[ghost]

Awesome, thanks a lot guys!

[zeroasterisk]

sweet - thanks!

[amadeus]

Since it looks like everything is good, closing.