-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 XDG_CONFIG_HOME dir for pwn.conf #1827
Conversation
b84a825
to
e9f96e7
Compare
I don't mind having this as an ADDITIONAL configuration that gets loaded, but we should keep |
pwnlib/update.py
Outdated
if should_check(): | ||
message = ["Checking for new versions of %s" % package_name] | ||
message += ["To disable this functionality, set the contents of %s to 'never' (old way)." % cache_file()] | ||
message += ["""Or add the following lines to ~/.pwn.conf (or /etc/pwn.conf system-wide): | ||
message += ["Or add the following lines to %s (or /etc/pwn.conf system-wide):" % (xdg_config_home + "/pwn.conf")] |
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.
List both locations, not one or the other.
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.
Also we need to handle the case where XDG_CONFIG_HOME is not set (e.g. macOS) and not recommend that path.
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.
Are you referring to ~/.config
vs $XDG_CONFIG_HOME
discussion above, or you're suggesting that all three of /etc/pwn.conf
, ~/.config/pwn.conf
and ~/.pwn.conf
should be mentioned here?
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.
all three of /etc/pwn.conf, ~/.config/pwn.conf and ~/.pwn.conf should be mentioned here
This, we should continue to support loading from ~/.pwn.conf even if $XDG_HOME/pwn.conf exists. If they both exist, load them both, don't really care about ordering.
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.
Probably ordering should be
/etc/pwn.conf
XDG_CONFIG_HOME/pwn.conf
~/.pwn.conf
This keeps the current behavior (~/.pwn.conf wins), while adding support for the XDG_CONFIG_HOME you're proposing.
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 suppose the macOS question was answered in the duscussion above?
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.
Yeah, if $XDG_CONFIG_HOME
is not set, then just don't try to load that file.
~/.pwn.conf support is not removed.
e9f96e7
to
92e9d72
Compare
Fixed CHANGELOG, merging |
~/.pwn.conf support is not removed.