-
Notifications
You must be signed in to change notification settings - Fork 52
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
Improve config file documentation #177
base: master
Are you sure you want to change the base?
Conversation
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.
Not sure how necessary I feel the example is vs. just elaborating a bit on the -C
option, but I don't mind it too much either.
swayidle.1.scd
Outdated
@@ -69,7 +69,7 @@ command line and in the config file. | |||
session after <timeout> seconds. Adding an idlehint event will also cause | |||
swayidle to call SetIdleHint(false) when run, on resume, unlock, etc. | |||
|
|||
All commands are executed in a shell. | |||
All commands are executed in a shell and must be quoted as they are parsed as a single argument |
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.
There is no requirement to quote commands in general, but commands with whitespaces need to be quoted to be parsed correctly as positional arguments. You don't need quotes to just run a script without arguments for example.
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.
Fair point. I'll update it.
swayidle.1.scd
Outdated
@@ -98,6 +98,16 @@ It will also lock your screen before your computer goes to sleep. | |||
To make sure swayidle waits for swaylock to lock the screen before it releases the | |||
inhibition lock, the *-w* options is used in swayidle, and *-f* in swaylock. | |||
|
|||
# EXAMPLE CONFIG | |||
|
|||
The same example as above, when specified in a config file suach as $XDG_CONFIG_HOME/swayidle/config |
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.
typo: s/suach/such/
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.
Ack, thanks
swayidle.1.scd
Outdated
@@ -98,6 +98,16 @@ It will also lock your screen before your computer goes to sleep. | |||
To make sure swayidle waits for swaylock to lock the screen before it releases the | |||
inhibition lock, the *-w* options is used in swayidle, and *-f* in swaylock. | |||
|
|||
# EXAMPLE CONFIG |
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.
Rather than a new section, maybe just write something like:
The events can alternatively be put into a configuration file as follows:
...
If a default location is chosen, the swayidle instantiation is then simply `swayidle -w`.
swayidle.1.scd
Outdated
@@ -11,7 +11,7 @@ swayidle - Idle manager for Wayland | |||
# OPTIONS | |||
|
|||
*-C* <path> | |||
The config file to use. By default, the following paths are checked in the following order: $XDG_CONFIG_HOME/swayidle/config, $HOME/.swayidle/config. | |||
The config file to use. By default, the following paths are checked in the following order: $XDG_CONFIG_HOME/swayidle/config, $HOME/.config/swayidle/config, $HOME/.swayidle/config. |
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.
$HOME/.config
is not a unique path being searched, but is the default value for XDG_CONFIG_HOME
when the environment variable is not explicitly set.
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, I wasn't sure how technical to get in the man page about precisely how this works, that in the absence of the env var, a default is injected into the top of the list... seems a bit wordy and unnecessary. I do think it's helpful to indicate that $HOME/.config
is the default value though. I mean I know that, but (a) many might not, and (b) I wasn't sure swayidle was doing it (due to other config file issues) until I read the code.
I'll try to clean this wording up a bit
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.
The default value of XDG_CONFIG_DIR
is part of the xdg base dir spec, so you don't have to mention anything - it's a bug if an application does not have that default. The original was fine.
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 know that, and I was still confused enough to motivate me to submit a PR. It's a small convenience found in many other man pages (see dmesg, top, wofi, wayfire for examples). What's wrong with it 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.
We don't describe this in our other manpages, and we prefer to not document external stuff which is better described elsewhere. For example, we don't document how POSIX shell command line parsing works for the event commands, how file permissions and ownership work, how environment variables function, etc.
When things are unclear, it is better to reference information than duplicate it. This helps ensure that it is understood to be a specification behavior, rather than a random app implementation detail. But that's a separate discussion that would involve PRs to more projects, so let's keep that out.
6f82cd9
to
3fc3b5d
Compare
Add some text to the note about commands being executed in a shell explaining that they need to be quoted or escaped if they contain whitespace or special characters.
Adds an example of config file contents to more clearly explain the format to users unfamiliar with it. It's the same example as already exists in the man page.
3fc3b5d
to
1107201
Compare
Small man page changes to make the documentation about using config files more clear. I had not quoted my commands in a config file at $HOME/.config/swayidle/config, and as such my
swaylock
parameters we're being ignored, although theswaylock
command itself was being executed. I went down several rabbit holes trying to figure out what was wrong, starting with discovering that the config file was in the wrong place according to the man page, since my$XDG_CONFIG_HOME
was not set. Only after symlinking$HOME/swayidle
to$HOME/.config/swayidle
did I figure out it was still reading the file correctly, and something else was wrong.Having the format of a config file laid out in an example, with an accurate list of possible locations, and explicitly mentioning that commands must be quoted to work would have been an immense help to me as a new user unfamiliar with swayidle.