From 9b6256d0fc62883efa31a0abd0cfee6f83eb4f51 Mon Sep 17 00:00:00 2001 From: David Adam Date: Thu, 10 Aug 2017 13:14:20 +0800 Subject: [PATCH 1/2] docs: improve set -Ux language and example By far the most common problem with universal variables being overridden by global variables is other values being imported from the environment; the `set -q; or set -gx` is much more of an edge case. --- doc_src/faq.hdr | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/doc_src/faq.hdr b/doc_src/faq.hdr index 320b4e91e..4b319e202 100644 --- a/doc_src/faq.hdr +++ b/doc_src/faq.hdr @@ -14,7 +14,7 @@ - How do I run a subcommand? The backtick doesn't work! - How do I get the exit status of a command? - How do I set an environment variable for just one command? -- Why doesn't `set -Ux` (exported universal vars) seem to work? +- Why doesn't `set -Ux` (exported universal varables) seem to work? - How do I customize my syntax highlighting colors? - How do I update man page completions? - Why does cd, pwd and other fish commands always resolve symlinked directories to their canonical path? @@ -126,19 +126,27 @@ begin end \endfish -\section faq-exported-uvar Why doesn't `set -Ux` (exported universal vars) seem to work? +\section faq-exported-uvar Why doesn't `set -Ux` (exported universal variables) seem to work? -Lots of users try to set exported environment variables like `EDITOR` and `TZ` as universal variables; e.g., `set -Ux`. That works but the behavior can be surprising. Keep in mind that when resolving a variable reference (e.g., `echo $EDITOR`) fish first looks in local scope, then global scope, and finally universal scope. Also keep in mind that environment vars imported when fish starts running are placed in the global scope. So if `EDITOR` or `TZ` is already in the environment when fish starts running your universal var by the same name is not used. +A global variable of the same name already exists. -The recommended practice is to not export universal variables in the hope they will be present in all future shells. Instead place statements like the following example in your config.fish file: +Environment variables such as `EDITOR` or `TZ` can be set universally using `set -Ux`. However, if +there is an environment variable already set before fish starts (such as by login scripts or system +administrators), it is imported into fish as a global variable. The variable scopes are searched from the "inside out", which +means that local variables are checked first, followed by global variables, and finally universal +variables. + +This means that the global value takes precedence over the universal value. + +To avoid this problem, consider changing the setting which fish inherits. If this is not possible, +add a statement to your user initilization file (usually +`~/.config/fish/config.fish`): \fish{cli-dark} -set -q EDITOR -or set -gx EDITOR vim +set -gx EDITOR vim \endfish -Now when fish starts it will use the existing value for the variable if it was in the environment. Otherwise it will be set to your preferred default. This allows programs like emacs or an IDE to start a fish shell with a different value for the var. This is effectively the same behavior seen when setting it as a uvar but is explicit and therefore easier to reason about and debug. If you don't want to allow using the existing environment value just unconditionally `set -gx` the var in your config.fish file. - \section faq-customize-colors How do I customize my syntax highlighting colors? Use the web configuration tool, `fish_config`, or alter the `fish_color` family of environment variables. From 7f1bdc5541ae721504748a3b431f9985a8bf0faa Mon Sep 17 00:00:00 2001 From: peoro Date: Wed, 9 Aug 2017 11:56:04 +0200 Subject: [PATCH 2/2] Improved warning message when exiting with jobs still active Fixes #4303 --- src/reader.cpp | 18 ++++++++++++++++-- tests/exit.expect | 19 +++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/reader.cpp b/src/reader.cpp index c46d13af8..1855fece9 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -2190,6 +2190,21 @@ bool shell_is_exiting() { return end_loop; } +static void bg_job_warning() { + fputws(_(L"There are still jobs active:\n"), stdout); + fputws(_(L"\n PID Command\n"), stdout); + + job_iterator_t jobs; + while (job_t *j = jobs.next()) { + if (!job_is_completed(j)) { + fwprintf(stdout, L"%6d %ls\n", j->pgid, j->command_wcstr()); + } + } + fputws(L"\n", stdout); + fputws(_(L"Use `disown PID` to let them live independently from fish.\n"), stdout); + fputws(_(L"A second attempt to exit will terminate them.\n"), stdout); +} + /// This function is called when the main loop notices that end_loop has been set while in /// interactive mode. It checks if it is ok to exit. static void handle_end_loop() { @@ -2214,8 +2229,7 @@ static void handle_end_loop() { } if (!data->prev_end_loop && bg_jobs) { - fputws(_(L"There are still jobs active (use the jobs command to see them).\n"), stdout); - fputws(_(L"A second attempt to exit will terminate them.\n"), stdout); + bg_job_warning(); reader_exit(0, 0); data->prev_end_loop = 1; return; diff --git a/tests/exit.expect b/tests/exit.expect index 3ff2074f7..6c0756675 100644 --- a/tests/exit.expect +++ b/tests/exit.expect @@ -9,8 +9,13 @@ expect_prompt send "sleep 111 &\r" expect_prompt send "exit\r" -expect "There are still jobs active" -expect "A second attempt to exit will terminate them." +expect -re "There are still jobs active:\r +\r + PID Command\r + *\\d+ sleep 111 &\r +\r +Use `disown PID` to let them live independently from fish.\r +A second attempt to exit will terminate them.\r" expect_prompt # Running anything other than `exit` should result in the same warning with @@ -18,8 +23,14 @@ expect_prompt send "sleep 113 &\r" expect_prompt send "exit\r" -expect "There are still jobs active" -expect "A second attempt to exit will terminate them." +expect -re "There are still jobs active:\r +\r + PID Command\r + *\\d+ sleep 113 &\r + *\\d+ sleep 111 &\r +\r +Use `disown PID` to let them live independently from fish.\r +A second attempt to exit will terminate them.\r" expect_prompt # Verify that asking to exit a second time does so.