-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Use XDG Base Directory Spec for default config file. #1061
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
Feature: XDG BaseDir Spec | ||
|
||
According to the XDG Base Directory Specification | ||
(http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html), | ||
if the XDG_CONFIG_HOME environment variable is set, this should be used | ||
to store application specific configuration. If unset, then a default of | ||
~/.config should be used. | ||
|
||
Background: | ||
Given I am in "dotfiles" git repo | ||
|
||
Scenario: XDG_CONFIG_HOME empty, store authorization in default directory | ||
Given the GitHub API server: | ||
""" | ||
require 'socket' | ||
require 'etc' | ||
machine_id = "#{Etc.getlogin}@#{Socket.gethostname}" | ||
|
||
post('/authorizations') { | ||
assert_basic_auth 'mislav', 'kitty' | ||
assert :scopes => ['repo'], | ||
:note => "hub for #{machine_id}", | ||
:note_url => 'http://hub.github.com/' | ||
json :token => 'OTOKEN' | ||
} | ||
get('/user') { | ||
halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' | ||
json :login => 'MiSlAv' | ||
} | ||
post('/user/repos') { | ||
halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' | ||
json :full_name => 'mislav/dotfiles' | ||
} | ||
""" | ||
Given $XDG_CONFIG_HOME is "" | ||
When I run `hub create` interactively | ||
When I type "mislav" | ||
And I type "kitty" | ||
And the file "../home/.config/hub" should contain "user: MiSlAv" | ||
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN" | ||
And the file "../home/.config/hub" should have mode "0600" | ||
|
||
Scenario: XDG_CONFIG_HOME is not empty, store authorization in specified directory | ||
Given the GitHub API server: | ||
""" | ||
require 'socket' | ||
require 'etc' | ||
machine_id = "#{Etc.getlogin}@#{Socket.gethostname}" | ||
|
||
post('/authorizations') { | ||
assert_basic_auth 'mislav', 'kitty' | ||
assert :scopes => ['repo'], | ||
:note => "hub for #{machine_id}", | ||
:note_url => 'http://hub.github.com/' | ||
json :token => 'OTOKEN' | ||
} | ||
get('/user') { | ||
halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' | ||
json :login => 'MiSlAv' | ||
} | ||
post('/user/repos') { | ||
halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' | ||
json :full_name => 'mislav/dotfiles' | ||
} | ||
""" | ||
Given $XDG_CONFIG_HOME is "../home/.xdgconfig" | ||
Given a directory named "../home/.xdconfig" | ||
When I run `hub create` interactively | ||
When I type "mislav" | ||
And I type "kitty" | ||
And the file "../home/.xdgconfig/hub" should contain "user: MiSlAv" | ||
And the file "../home/.xdgconfig/hub" should contain "oauth_token: OTOKEN" | ||
And the file "../home/.xdgconfig/hub" should have mode "0600" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import ( | |
"github.com/github/hub/Godeps/_workspace/src/github.com/howeyc/gopass" | ||
"github.com/github/hub/ui" | ||
"github.com/github/hub/utils" | ||
"github.com/github/hub/Godeps/_workspace/src/github.com/cep21/xdgbasedir" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of pulling in a new dependency, couldn't we simply read the value of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I'll do it later today if that's preferred. I just figured that it might be nice to have the xdgbasedir spec available in it's entirety, and why not pull it in from a library?. For instance there is an argument that passwords should go in data not in config, and the dependency has code for data directories as well. Hub config would go in the config directory. If a user wants to cache their password, but not save it permanently that could go in the cache directory etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My general rule for software is not to pull in dependencies when you can simply inline something. This is the entirety of the logic that we're obtaining by pulling in an external library: // DataHomeDirectory is a single base directory relative to which user-specific data files should be written
func DataHomeDirectory() (string, error) {
return getInEnvOrJoinWithHome("XDG_DATA_HOME", defaultDataHomeDirectory)
} we can do the same ourselves. Also, this library uses |
||
) | ||
|
||
var defaultConfigsFile string | ||
|
@@ -29,7 +30,12 @@ func init() { | |
utils.Check(fmt.Errorf("Can't get current user's home dir")) | ||
} | ||
|
||
defaultConfigsFile = filepath.Join(homeDir, ".config", "hub") | ||
// Config file using XDG Base Directory spec | ||
defaultConfigsDir, xdgerror := xdgbasedir.ConfigHomeDirectory() | ||
if xdgerror != nil { | ||
utils.Check(xdgerror) | ||
} | ||
defaultConfigsFile = filepath.Join(defaultConfigsDir, "hub") | ||
} | ||
|
||
type yamlHost struct { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this section is at all necessary. We can just mention somewhere that hub respects XDG_CONFIG_HOME, and that should be enough for people familiar with XDG to understand what's going on. We can keep referring to
~/.config/hub
in the rest of the documentation as before.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just moved house and have had intermittent internet access. I thought I'd get a chance to do this before I moved, but it didn't happen. Lucky this is not very important 😄 I'll sort it out when I get a chance over the next couple of days.