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

Support @ as an alternative name version separator #2307

Closed
sheerun opened this issue Jun 10, 2016 · 14 comments
Closed

Support @ as an alternative name version separator #2307

sheerun opened this issue Jun 10, 2016 · 14 comments
Assignees

Comments

@sheerun
Copy link
Contributor

sheerun commented Jun 10, 2016

Migrated from bower/endpoint-parser


The current # doesn't work unquoted in ZSH which makes it annoying to use. It's also inconsistent with npm which uses @ (for that exact reason?).

Add support for using both @ and # when doing eg. bower install

bower install jquery#2
bower install jquery@2

Are you sure? Dependencies that will use this format won't be backward-compatible.


Just for installing in the CLI, not bower.json.

@ubermuda
Copy link

It would be more clear and less wtf-prone if only one version separator was supported. I can already see people used to "bower install jquery@2 --save" trying to add "somethingsomething@3" in their bower.json.

It would be a BC-breaking change indeed, but it could be introduced in 2.x. After all, what are major version changes for if not for BC?

If you're worried about the migration path, double support could be in 1.8 (or 1.9), with a deprecation warning encouraging people to migrate their config file to @

@sheerun
Copy link
Contributor Author

sheerun commented Jun 20, 2016

For now let's just allow it in CLI, undocumented, without any deprecations

@doomedramen doomedramen self-assigned this Jun 28, 2016
@sheerun
Copy link
Contributor Author

sheerun commented Jun 28, 2016

It's in the main repository now (bower/bower/packages)

@doomedramen
Copy link
Member

Thanks, I am looking at multiple solutions to this ticket,
for example, it would be possible to just modify bower-endpoint-parser to replace @ with # when parsing the endpoint, this would mean that all logs would still show #, the alternative would require a major overhaul on the code.

@sheerun
Copy link
Contributor Author

sheerun commented Jun 28, 2016

It's ok to just modify "bower install" command to replace @ with #, no need to modify parsing logic.

@doomedramen
Copy link
Member

doomedramen commented Jun 28, 2016

Im happy to that but there are other commands (such as bower info) that could be given a @. Wouldn't it make sense to allow all the commands to accept @ if install does?

@sheerun
Copy link
Contributor Author

sheerun commented Jun 28, 2016

Ach, you're right. Maybe it's better to modify bower-endpoint-parser like you suggested :)

@doomedramen
Copy link
Member

If we change it there it will trickle down to everything else, it would also mean that if this gets made a 'thing' we could store the separator value (@ or #) in the endpoint object so it could be used in logs, bower.json, etc.
Not sure how to best write a test for this (based on the current test setup) but ill figure something out.

@sheerun
Copy link
Contributor Author

sheerun commented Jun 28, 2016

Probably an integration test in test/commands/install.js would be the best. Maybe plus one unit test in parser itself

@doomedramen
Copy link
Member

Due to the decompose function also being used for git urls and other strings that are designed to have @ symbols in, I will do as originally planned and replace inside the commands. this can be replaced if/when documented support if brought in.

@sheerun
Copy link
Contributor Author

sheerun commented Jul 1, 2016

agreed

@doomedramen
Copy link
Member

I have put a pull request in the using @ within the info command #2320

Just working on testing the install command, the current test (helper) setup does not seem to allow using # to specify a version of a 'fake' dependency.

@doomedramen
Copy link
Member

Merge replaced with #2322

@doomedramen
Copy link
Member

Completed with #2322

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

No branches or pull requests

3 participants