Closed Flimm closed 1 year ago
I think this change will make it even easier for users to find the configuration file.
Escaping the filename is useful. I like that.
Adding the os.path.join
is somewhat helpful (providing the full path), at the expense of making the command significantly longer (and thus more difficult to type if needed). Isn't it preferable since the name has already been prescribed to simply help the user locate the containing directory?
On some systems with Python 3, only the
python3
executable is available (and notpython
), so I changed the executable topython3
as well. This project doesn't support Python 2 any more.
I'd really like to get away from the aberration which is "python3". That name doesn't exist on Windows. I'd consider using py
(python-launcher) as that exists on both platforms. Otherwise, I'd prefer to stick to the version-agnostic name (python), which can (and in my opinion should) refer to the best available Python.
This was my first-time experience. I ran this command that I found in the documentation:
$ python3 -c "import keyring.util.platform_; print(keyring.util.platform_.config_root())"
/home/flimm/.config/python_keyring
I then tried to create a file named python_keyring
in the .config
directory. I didn't read the previous paragraph in the documentation properly. It took me a while to understand why my custom configuration was having no effect. I contributed this proposed change to make the documentation harder to misunderstand in the way that I did.
With regards to running python3
or python
, Python's documentation says that the binary is expected to be python3
. However, on Windows, python3
doesn't exist. I've asked a question about this on Stack Overflow: https://stackoverflow.com/q/75885628/247696
Probably if we want to make it easy to emit the entire config path, that logic should probably be embedded in the library, rather than adding long recipes for copy paste in the docs. Maybe a keyring diagnose
command or keyring config
command.
That's fine by me. I think my pull request is minor and doesn't merit more time spent on discussion. I'm happy for you to make the final call.
Thanks for understanding. I'll decline this change for the reasons stated above, but plan to follow up with something better in #633.
This is a small tweak. With the old documentation, I created a file named
/home/song/.config/python_keyring
and I wondered why it failed silently. I realise that I should have read the existing documentation more carefully. I think this change will make it even easier for users to find the configuration file.On some systems with Python 3, only the
python3
executable is available (and notpython
), so I changed the executable topython3
as well. This project doesn't support Python 2 any more.