Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--config-set installed_paths overwrites itself #1436

Closed
photodude opened this issue Apr 28, 2017 · 6 comments
Closed

--config-set installed_paths overwrites itself #1436

photodude opened this issue Apr 28, 2017 · 6 comments

Comments

@photodude
Copy link
Contributor

photodude commented Apr 28, 2017

phpcs 2.8.1
attempted

phpcs --config-set installed_paths /path/to/one
phpcs --config-set installed_paths /path/to/two

result

only /path/to/two was added to the installed_paths

expected

both /path/to/one and /path/to/two should be added to the installed_paths.

note

phpcs --config-set installed_paths /path/to/one,/path/to/two will add both to the installed_paths. but in the other case it's unexpected to lose /path/to/one by using a second call to --config-set

@gsherwood
Copy link
Member

I take your point, but this is working as expected and I don't intend to make any changes to it at this time.

The config settings are single values and the config-set and config-delete commands act on an entire config value at once.

If config-set added sub-values to a config value instead of overwriting it, I'd need to provide a new command that allowed you to remove those sub-values because config-delete wipes the entire config value. This fundamentally changes how config values are represented as I would now have a special type of config value that accepts (presumably) an array.

@photodude
Copy link
Contributor Author

I'm looking at it from the case of installing multiple coding standards where each coding standard uses the composer scripts post-install to run config-set.

With the current way the config-set works in this case only the last installed coding standard would be installed.

I can understand if it's low priority but I believe this would be a good improvement.

@jrfnl
Copy link
Contributor

jrfnl commented Apr 30, 2017

AFAIK Composer will not run scripts from dependencies.
The main project has to define it's own script to set the installed_paths config variable and can do so for all standards pulled in as dependencies in one go.

@photodude
Copy link
Contributor Author

photodude commented Apr 30, 2017

As you mentioned, if the package was installed as global the scripts would run. Which could/would cause the case of multiple coding standards overwriting the --config-set installed_paths

@jrfnl
Copy link
Contributor

jrfnl commented May 3, 2017

@photodude You may want to have a look at the phpcodesniffer-composer-installer for Composer and at the question about this which I asked them: PHPCSStandards/composer-installer#24

@gsherwood
Copy link
Member

Closing as nothing has happened with this proposal. Keeping config-set the same for all config values is still preferred at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants