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

Added support for user defined git shorthand #278

Closed

Conversation

indieisaconcept
Copy link

As per /issues/276 here is a pull request to support overriding the shorthand used when resolving git urls.

.bowerrc

{
    ...
    "shorthand": "git://somehost.com/"
    ...
}

Once set rather that using "git://github.com/", "git://somehost.com/" is used as the shorthand.

> bower install jquery/jquery
> bower cloning git://somehost.com/jquery/jquery.git
> bower caching git://somehost.com/jquery/jquery.git
> ......

@indieisaconcept
Copy link
Author

Hmmm build seems to be failing on "Should extract tar and zip files from normal URL packages"

@satazor
Copy link
Member

satazor commented Feb 25, 2013

Thanks @indieisaconcept, this makes sense. The travis fails sometimes, dunno why. It always passes in my local computer. Can you also update the README?

@sindresorhus
Copy link
Contributor

I restarted the build and it passes now. Heroku is sometimes really slow and times out...

@necolas
Copy link
Contributor

necolas commented Feb 25, 2013

Please could this use a better key than "shorthand"? Something like remote or server_root.

@satazor
Copy link
Member

satazor commented Feb 25, 2013

shorthand_hostname? Having shorthand is nice because this config option only affects how shorthand endpoints are expanded, such as twitter/flight -> git://github.com/twitter/flight.git

@indieisaconcept
Copy link
Author

@satazor I'll update the pull request later today with a README update and a config key of "shorthand_hostname".

@satazor
Copy link
Member

satazor commented Feb 25, 2013

I'm not sure about the hostname part, specially since the protocol (git://) is included. Let me think..

@satazor
Copy link
Member

satazor commented Feb 25, 2013

Maybe we can allow a hogan template string to be defined in that option like so:

'shorthand_expander': 'git://github.com/{{ organization }}/{{ package }}.git'

What you guys think?
This would give us much more flexibility, because an user might want to specify an URL that is completely different.
Having this in mind, shorthand_expander might be a good name.

@sindresorhus
Copy link
Contributor

@satazor he's not asking for being able to customize org and package, but rather the host.

I also don't see the point, since all GitHub instances has the same format.

@satazor
Copy link
Member

satazor commented Feb 25, 2013

In his case yes, but other users might want to be able to specify completely different ones. Let's say that I have my own git service, and want to make all expansions mapped to git://myservice/organization/repos/package-name.git. This would allow it while the protocol + host prefix don't.
I'm just taking a different approach to the same problem, possibly giving more flexibility to others that might encounter the same wall.

@mehcode
Copy link

mehcode commented Feb 25, 2013

To throw in my 2 cents.., I'd want the ability to define the hogan template to be able to map to non-github private instances or something. Flexibility is good.

I'd go for the shorthand name though. Just think of it like the 'shorthand' template.

'shorthand': 'git://organization.example.com/git/{{ organization }}/repos/{{ package }}.git'

@sindresorhus
Copy link
Contributor

@satazor you should have started with your second comment. +1 from me.

@satazor
Copy link
Member

satazor commented Feb 25, 2013

@mehcode thanks for the feedback, good to know you like the hogan template idea. Don't you think shorthand is too vague?

@satazor
Copy link
Member

satazor commented Feb 25, 2013

@sindresorhus you're right, I was being lazy heh

@necolas
Copy link
Contributor

necolas commented Feb 25, 2013

I'm not sure this needs to be complicated yet, though.

For now, not having a hard-coded remote URL is a good start and provides the same functionality that Component(1) does (using the remotes key).

Dealing with a completely different format might be useful at some point. But who is to say that everyone would always use the same "name/name" pattern? It's a bigger issue.

And FWIW, shorthand is not only vague but a confusing key name given it's value pair.

@mehcode
Copy link

mehcode commented Feb 25, 2013

It's in a configuration file. So the keys are describing what the values are configuring. So a key shorthand expects a value to configure what the shorthand functionality does. At least that how I see it.

And we would only need to disambiguate if there was another configuration option related to the shorthand form in the bowerrc file.

Dealing with a completely different format might be useful at some point. But who is to say that everyone would always use the same "name/name" pattern? It's a bigger issue.

Good point. We could perhaps define different sources based on input filters and output templates. Bower could be just a name and github could be defined as name/repo, etc. Each source could have a name and could be passed to bower specifically like bower --source bower (default) or bower --source github. Just an idea though.

@satazor
Copy link
Member

satazor commented Feb 25, 2013

Dealing with a completely different format might be useful at some point. But who is to say that everyone would always use the same "name/name" pattern? It's a bigger issue.

org/repo could be split automatically and sent as organization & package to the template. The full endpoint (org/repo) could also be sent to the template. For some edge cases we could support a function as a string (rc files are json and don't allow functions). Examples:

'shorthand_expander': 'git://github.com/{{ organization }}/{{ package }}.git'
'shorthand_expander': 'git://github.com/{{ endpoint }}.git'
'shorthand_expander': 'function (endpoint) { return "git://github.com/" + endpoint + ".git"; }'

I don't think we should support the function as a string for now. Though we got a possible solution if someone ever requests it.

@indieisaconcept
Copy link
Author

If the general consensus is template expansion would be useful I'm happy to extend my pull request to incorporate this. Just need agreement on the key name.

Quite like @mehcode's reasoning for using "shorthand" as the key though.

It's in a configuration file. So the keys are describing what the values are configuring. So a key shorthand expects a value to configure what the shorthand functionality does. At least that how I see it.

And we would only need to disambiguate if there was another configuration option related to the shorthand form in the bowerrc file.

But "'shorthand_expander'" is very explicit to the user that some expansion will likely occur.

Suitable documentation could alleviate user confusion whatever the key is.

@satazor
Copy link
Member

satazor commented Feb 26, 2013

shorthand_resolver is also a good name

@indieisaconcept
Copy link
Author

Ok just rebased my pull with the changes outlined above.

  • Key "shorthand_resolver"
  • Template support via hogan
  • Updated README

@indieisaconcept
Copy link
Author

@satazor thanks for the feedback will pick these up after work tonight.

@satazor
Copy link
Member

satazor commented Feb 26, 2013

@indieisaconcept sorry for being picky.

@indieisaconcept
Copy link
Author

@satazor np, I appreciate the need for coding standards and your right my example could be clearer.

@indieisaconcept
Copy link
Author

@satazor changes added as per feedback.

@satazor satazor closed this in 339831a Feb 28, 2013
@satazor
Copy link
Member

satazor commented Feb 28, 2013

Thanks @indieisaconcept !

@indieisaconcept
Copy link
Author

happy to help

@davidmaxwaterman
Copy link
Contributor

HI...just wanted to comment here that I have a need to be able to change the URLs so that bower uses one of the other two styles of URL for github. IINM, there are three styles 👍

http : https://github.com/01org/webapps-scientific-calculator.git
ssh : git@github.com:01org/webapps-scientific-calculator.git
git read-only : git://github.com/01org/webapps-scientific-calculator.git

In my network environment, the 'git read-only' one does not work (or needs some extra work done that is outside my area of expertise) - I tend to use ssh for some reason, but http also works. So, to get bower to work, I figured I'd try to use one of the others, eg :

"shorthand_resolver": "https://github.com/{{{ organization }}}/{{{ package }}}.git"

I am hoping this will work, but unfortunately node installs the previous version bower so I have to find out how to install the latest version :/ (Any pointers welcome)

@satazor
Copy link
Member

satazor commented Apr 22, 2013

@davidmaxwaterman

1 - clone bower to a folder
2 - npm install inside it
3 - sudo npm link

This should do it.

@satazor satazor mentioned this pull request Apr 22, 2013
@davidmaxwaterman
Copy link
Contributor

thanks for the help...can you tell me if :

"shorthand_resolver": "https://github.com/{{{ organization }}}/{{{ package }}}.git"

is supposed to work? I can't get it to do anything at different to the previous behaviour :/ What file is it supposed to go in? I've tried .bowerrc in the project's root, and also in the bower.json file, but it still loads git://github.com/ urls :/

@satazor
Copy link
Member

satazor commented Apr 22, 2013

@davidmaxwaterman the shorthand resolver is an expander for bower install organization/package e.g. bower install twitter/bootstrap will expand to bower install git://github.com/twitter/bootstrap

I think you want another thing, see: http://stackoverflow.com/questions/1722807/git-convert-git-urls-to-http-urls

@davidmaxwaterman
Copy link
Contributor

Rewriting the URL using the git setting worked. I'm still not clear why the expander is limited to end points specified on the command line, but I'm probably misunderstanding something.
Thanks for the git tip.

@radicand
Copy link

@davidmaxwaterman you may have run into something I did just now, which is that the shorthand_resolver has too low a priority in the list to get hit if you are trying to use semvers.

From the code, having a semver will prevent it:

} else if (semver.validRange(endpoint)) {

What I've done in my case is to specify the spec like

"mypackage": "git@github.com/myprivateorg/mypackage#~0.1.0"

which works fine. https:// prefixes should also work this way.

@davidmaxwaterman
Copy link
Contributor

ok, thanks...the git tip worked for me anyway - thanks :)

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

Successfully merging this pull request may close these issues.

7 participants