[Enhancement] Clean up i3-nagbar execution and configuration of user's preferred terminal.

[MachFour]

Output of i3 --version:
i3 version 4.13 (2016-11-08) © 2009 Michael Stapelberg and contributors

Hi, I came across this issue the other day when I was testing out a modification of my favourite terminal emulator (termite), to how it executes commands. This modified termite was my default (and only) terminal emulator.

I use i3-nagbar as a shutdown dialog to exit i3, but suddenly that seemed to no longer work with the modified termite installed. Looking through the source code, I found that i3-nagbar executes commands via a hardcoded call to i3-sensible-terminal (see here), which relies on the precise argument parsing behaviour of terminal emulators to work properly. Now, this also made me wonder if the behaviour of i3-sensible-terminal could also be improved.

I saw that there has been some discussion about these issues in #2199. I did some thinking and have come up with the following solution:

1. I think that i3-nagbar's command execution method should be reworked to avoid the hidden dependency on terminal emulator functionality, and instead call /bin/sh directly with the command passed using -b. (It would also remove the strange terminal flickering seen when using i3-nagbar to execute commands).
2. Any time that i3 wants to call a 'sensible terminal emulator', I think that we should introduce a way for users to exactly specify how they want that terminal emulator to be called. To this end, we could introduce a config file option to specify the user's preferred terminal emulator, and method to execute commands with it. For example, $set terminal_exec_cmd = "xfce4-terminal -x %s", where %s is replaced by the (space separated) command line to execute. Or if the terminal requires quotes, it could be e.g. $set terminal_exec_cmd = "termite -e '%s'".
   When i3-sensible-terminal is called, or would be called, the terminal_exec_cmd variable is used in preference to one from the default list of terminals. For a 'fresh install' where that variable would not yet be set, the behaviour would be the same as it is now. It would still be up to packagers to check that the default settings are sane.

Example:
As I understand it, the original use-case for i3-nagbar was to get users to fix their config file if they execute i3 with errors in it. Therefore, the new -b arguments in this situation would be i3-sensible-terminal -e i3-sensible pager <logfile> and i3-sensible-terminal -e i3-sensible-editor <configfile>.

If this enhancement (or variant of) is accepted, I am very happy to work on a PR.

Cheers,
Max

[Airblader]

Thanks for taking the time to write down your thoughts in such detail!

I'm not happy about adding new configuration for this because I don't think that i3 should have any knowledge about what terminal emulator to call (and by extension, how to call it). In general i3 should only call the system shell or tools included with i3 (such as i3-nagbar).

What are your thoughts on my proposal of just teaching i3-nagbar a flag to not use i3-sensible-terminal but pass the command to /bin/sh instead? It still feels like the cleanest approach to me.

By the way, in general I'd just recommend using something other than i3-nagbar for actual navigational tools. Launchers like dmenu and rofi are much better suited for such tasks, IMHO.

[MachFour]

Hey,

My thinking on the terminal config was that it's easier than trying to make sure that i3-sensible-terminal (and friends) work well on pretty much any distro with any terminal the user wants/has installed. In a sense, i3 uses implicit knowledge about what system tools the user does have, but this knowledge is hardcoded into i3-sensible-{terminal,editor,pager}. That being said, I can understand your reluctance to bring it in as a config option.

I do like your idea of having two separate flags in i3-nagbar, one for a shell action and one for a terminal action. It would indeed be a lot simpler, and a lot cleaner in that the changes are confined just to i3-nagbar. Perhaps it could be modified to look something like this?

i3-nagbar [-m <message>] [-bt <button> <terminal-action>] [-bs <button> <shell-action>] [-t warning|error] [-f <font>] [-v]

Again, I'm very happy to work on implementing this.

Regarding other tools like dmenu - I do in fact use i3-dmenu-desktop/dmenu-frecency as my launcher. But for simple tasks where you just want to confirm launching of one or two scripts, i3-nagbar is a pleasantly simple tool, and has the bonus of coming pre-packaged with i3. So in general it's quite appealing to use ;)

[Airblader]

Perhaps it could be modified to look something like this?

That's mostly OK with me, but we have to make sure the change is backwards-compatible, i.e., we cannot remove the current flag.

i3-nagbar is a pleasantly simple tool

… until you need things it can't do like here. That's exactly the downside of simplicity. :-)

and has the bonus of coming pre-packaged with i3

Installing another package really isn't that much trouble. :-) Especially if you are already using these other tools anyway.

Again, I'm very happy to work on implementing this.

Go ahead!

[stapelberg]

I’m thinking about reverting this. i3-nagbar is to be used only by i3, and as such should only contain the features we absolutely need in i3. Adding features upon user request for non-i3-internal use-cases is a slippery slope.

@Airblader Is this okay with you?

[Airblader]

Fine with me, although admittedly a bit of a mean move to revert it a few months after merging without it ever having causes problems. But once we release it'd be a breaking change, so I understand the sentiment.

[stapelberg]

I agree that it’s not a great experience for @MachFour, and I’m sorry about that.

I only saw the commit now, hence the late comment on it. I’ll revert it in a second in preparation for the upcoming release.

[MachFour]

Hey all,
I can accept the reversion, but I will say that part of the reason I wanted this is that it actually reduces i3-nagbar's reliance on the workings of external tools, i.e. as required by i3-sensible-termimal.
In other words, if your terminal stops working for some reason, I don't think it should break i3-nagbar from being able to run commands.

If there's another solution to this, that would be ideal. If not, then so be it.

Cheers

[stapelberg]

AFAICT, we’re calling i3-nagbar with the following actions:

i3/src/bindings.c

Lines 845 to 859 in 4dca8e6

	char *pageraction;
	sasprintf(&pageraction, "i3-sensible-pager \"%s\"\n", errorfilename);
	char *argv[] = {
	NULL, /* will be replaced by the executable path */
	"-f",
	config.font.pattern,
	"-t",
	"error",
	"-m",
	"The configured command for this shortcut could not be run successfully.",
	"-b",
	"show errors",
	pageraction,
	NULL};
	start_nagbar(&command_error_nagbar_pid, argv);

i3/src/config_parser.c

Lines 791 to 810 in 4dca8e6

	char *editaction, *pageraction;
	sasprintf(&editaction, "i3-sensible-editor \"%s\" && i3-msg reload\n", configpath);
	sasprintf(&pageraction, "i3-sensible-pager \"%s\"\n", errorfilename);
	char *argv[] = {
	NULL, /* will be replaced by the executable path */
	"-f",
	(config.font.pattern ? config.font.pattern : "fixed"),
	"-t",
	(has_errors ? "error" : "warning"),
	"-m",
	(has_errors ? "You have an error in your i3 config file!" : "Your config is outdated. Please fix the warnings to make sure everything works."),
	"-b",
	"edit config",
	editaction,
	(errorfilename ? "-b" : NULL),
	(has_errors ? "show errors" : "show warnings"),
	pageraction,
	NULL};
	start_nagbar(&config_error_nagbar_pid, argv);

All of the actions we specify can reasonably require to be run in a terminal: if the user uses a graphical editor or pager, then that’s unnecessary, but in many cases, a textual editor or pager will be used.

Hence, if your terminal stops working, you’re out of luck, and there’s nothing that i3-nagbar can do about it, even with your change.

Did I miss something here?