Made `flymake-phpcs-standard' safe-local-variable #3
Conversation
|
Not sure about this one: flymake-phpcs-standard allows you to set a custom standard, and via that, set custom plugins for phpcs, which basically means that it can be used to execute arbitrary PHP code as the current user when they edit a file under your control. Making it buffer-local is something I've been meaning to do for a bit, so I'll cherry-pick those bits for the next release. |
|
Ah yeah ... because `flymake-phpcs-standard' can be set to a path pointing to a standard, right? So in theory this could do it: (put 'flymake-phpcs-standard 'safe-local-variable 'string-or-null-but-not-a-path-p)if `string-or-null-but-not-a-path-p' existed as a function? Btw. It never worked for me to let `flymake-phpcs-standard' be a path to a standard - but I didn't try hard either. |
Yep.
Yes, that sounds about right. You should be able to drop a lambda in there if you wanted. I've also just noticed that you're using
Hmm, curious, I use it on a daily basis for a custom company coding style. Have you been successful in using the standard when manually using |
I tried that in f3d8aac.
Hmm... I don't think so... Putting it as property on the variable is fine/better I think.
Now I tried a bit harder. I can't run it from the commandline either. I ran into this issue squizlabs/PHP_CodeSniffer#16 which is fixed in the next version of phpcs. |
|
Awesome, looks good, and you're right about the Thanks for the work on this, I'll try to find time tonight to get it merged, tested and released. |
|
I haven't forgotten about this, I promise, unfortunately it's been crunch time at work and I haven't had time to get around to it. I had an issue with backwards-compat when I applied the code locally, in that my existing I haven't had time to check to see if there's a way to avoid breaking compat in this way, it's a fairly minor issue but I'd rather avoid it if I can. Hopefully things will ease up around here soon and I'll get chance to get this dusted off and merged in. |
|
Sorry for the delay. I've created a new branch Its a bit rough around the edges still and currently requires my fork of Flymake as I haven't added compat for the original Flymake yet. It'd be useful if you could take a look and see if it meets your needs or if you have any concerns that still need addressing with it. |
|
Hey @arnested, just a quick ping, did this ever resolve the issue for you? |
|
I haven't really had the time to look deeper into it. From the first look I think it is overly complex and maybe too complex "just" for maintaining backwards compatibility. I'm not sure there is a simpler way to achieve that though. |
|
Ah, to be clear, most of the extra complexity would be needed with your original patch too - dir local variables get loaded after the buffer completes loading, which means there's already a flymake syntax check started using the non-local value of the variable. So, there's some hoop-jumping to cancel any running check and kick off a new one. |
`flymake-phpcs-standard' is only safe as a local variable if the specified standard is not pointing to a file/path to a standard. So the safe-local-variable property is adjusted to only be safe when the value is string-or-null-p AND it is not file-exists-p. This might fail in the case where you specify i.e. PEAR as standard and also have a file or folder named PEAR relative to the buffer. PHPCS will choose the installed standard but `flymake-phpcs-standard' will consider the setting unsafe. At least it only fails to the safe side - it won't allow unsafe values but can mark a few safe values as unsafe. Another approach would be to only accept all installed standards (phpcs -i) as safe values but we'll spare the overhead of running external commands.
|
Revisiting this after one and a half year I think the change is as simple as 033505c. I think this should be backwards compatible. |
This makes it possible to set a specifik standard in a local variables
block at the end of a file or in a dir-locals.el.