Merge branch 'no_percent'

Drops the % notation for process expansion. The existing notation was a
mess and expanded jobs, process ids, and process names via dark magic.
With this change, % is no longer a special character and can be used
unescaped with impunity.

The variables %self and %last, referring to fish's own pid and the pid
of the last backgrounded job respectively, have been replaced with $pid
and $last_pid. These are read-only variables, protected against being
redefined by the user.

Author's note: I would have personally preferred $fish_pid instead of
$pid but since we debated changing $version to $fish_version and then
reverted that change (with much acrimony), it makes no sense to break
with that precedent here. Additionally, $fish_last_pid is quite wordy.

Closes #4230. Closes #1202.

CHANGELOG.md

@@ -12,6 +12,7 @@ This section is for changes merged to the `major` branch that are not also merge
 - For loop control variables are no longer local to the for block (#1935).
 - A literal `{}` now expands to itself, rather than nothing. This makes working with `find -exec` easier. (#1109, #4632)
 - Successive commas in brace expansions are handled in less surprising manner (`{,,,}` expands to four empty strings rather than an empty string, a comma and an empty string again). (#3002, #4632).
+- `%` is no longer used for process and job expansion. `$pid` and `$last_pid` have taken the place of `%self` and `%last` respectively. (#4230, #1202)
 
 ## Notable fixes and improvements
 - `wait` builtin is added for waiting on processes (#4498).

po/de.po

@@ -2118,7 +2118,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

po/en.po

@@ -2068,7 +2068,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

po/fr.po

@@ -2185,8 +2185,8 @@ msgstr "$? n’est pas le code de retour. Dans fish, veuillez utiliser $status."
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
-msgstr "$$ n’est pas le PID. Dans fish, veuillez utiliser %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
+msgstr "$$ n’est pas le PID. Dans fish, veuillez utiliser $pid."
 
 #: src/parse_constants.h:278
 msgid "$# is not supported. In fish, please use 'count $argv'."

po/nb.po

@@ -2027,7 +2027,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

po/nn.po

@@ -2027,7 +2027,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

po/pl.po

@@ -2051,9 +2051,9 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
-"$$ nie jest numerem identyfikacyjnym procesu. W fish używane jest %%self."
+"$$ nie jest numerem identyfikacyjnym procesu. W fish używane jest $pid."
 
 #: src/parse_constants.h:278
 msgid "$# is not supported. In fish, please use 'count $argv'."

po/pt_BR.po

@@ -2084,7 +2084,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

po/sv.po

@@ -2031,7 +2031,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

po/zh_CN.po

@@ -2045,7 +2045,7 @@ msgstr ""
 
 #: src/parse_constants.h:275
 #, c-format
-msgid "$$ is not the pid. In fish, please use %%self."
+msgid "$$ is not the pid. In fish, please use $pid."
 msgstr ""
 
 #: src/parse_constants.h:278

share/functions/__fish_complete_pids.fish

@@ -1,11 +1,10 @@
 function __fish_complete_pids -d "Print a list of process identifiers along with brief descriptions"
     # This may be a bit slower, but it's nice - having the tty displayed is really handy
     # 'tail -n +2' deletes the first line, which contains the headers
-    #  %self is removed from output by string match -r -v
-    set -l SELF %self
+    #  $pid is removed from output by string match -r -v
 
     # Display the tty if available
     # But not if it's just question marks, meaning no tty
-    ps axc -o pid,ucomm,tty | string match -r -v '^\s*'$SELF'\s' | tail -n +2 | string replace -r ' *([0-9]+) +([^ ].*[^ ]|[^ ]) +([^ ]+) *$' '$1\t$2 [$3]' | string replace -r ' *\[\?*\] *$' ''
+    ps axc -o pid,ucomm,tty | string match -r -v '^\s*'$pid'\s' | tail -n +2 | string replace -r ' *([0-9]+) +([^ ].*[^ ]|[^ ]) +([^ ]+) *$' '$1\t$2 [$3]' | string replace -r ' *\[\?*\] *$' ''
 end
 

share/functions/edit_command_buffer.fish

@@ -6,9 +6,9 @@ function edit_command_buffer --description 'Edit the command buffer in an extern
     else
         # We should never execute this block but better to be paranoid.
         if set -q TMPDIR
-            set f $TMPDIR/fish.(echo %self).fish
+            set f $TMPDIR/fish.$pid.fish
         else
-            set f /tmp/fish.(echo %self).fish
+            set f /tmp/fish.$pid.fish
         end
         touch $f
         or return 1

share/functions/suspend.fish

@@ -18,11 +18,11 @@ function suspend --description 'Suspend the current shell.'
     end
 
     if status is-interactive
-        echo -ns 'Suspending ' %self ': run'
-        echo -n (set_color --bold) 'kill -CONT' %self (set_color normal)
+        echo -ns 'Suspending ' $pid ': run'
+        echo -n (set_color --bold) 'kill -CONT' $pid (set_color normal)
         echo 'from another terminal to resume'
     end
 
     # XXX always causes a zombie until one fg's when we do this:
-    kill -STOP %self
+    kill -STOP $pid
 end

src/common.cpp

@@ -1002,7 +1002,6 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring
                 case L'|':
                 case L';':
                 case L'"':
-                case L'%':
                 case L'~': {
                     if (!no_tilde || c != L'~') {
                         need_escape = 1;
@@ -1316,12 +1315,6 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                     }
                     break;
                 }
-                case L'%': {
-                    if (unescape_special && (input_position == 0)) {
-                        to_append_or_none = PROCESS_EXPAND;
-                    }
-                    break;
-                }
                 case L'*': {
                     if (unescape_special) {
                         // In general, this is ANY_STRING. But as a hack, if the last appended char

src/env.cpp

@@ -322,7 +322,7 @@ bool string_set_contains(const T &set, const wchar_t *val) {
 
 /// Check if a variable may not be set using the set command.
 static bool is_read_only(const wchar_t *val) {
-    const string_set_t env_read_only = {L"PWD", L"SHLVL", L"_", L"history", L"status", L"version"};
+    const string_set_t env_read_only = {L"PWD", L"SHLVL", L"_", L"history", L"status", L"version", L"pid"};
     return string_set_contains(env_read_only, val);
 }
 
@@ -969,6 +969,9 @@ void env_init(const struct config_paths_t *paths /* or NULL */) {
     wcstring version = str2wcstring(get_fish_version());
     env_set_one(L"version", ENV_GLOBAL, version);
 
+    // Set the $pid variable (%self replacement)
+    env_set_one(L"pid", ENV_GLOBAL, to_string<long>(getpid()));
+
     // Set up SHLVL variable. Not we can't use env_get because SHLVL is read-only, and therefore was
     // not inherited from the environment.
     wcstring nshlvl_str = L"1";

src/exec.cpp

@@ -1149,6 +1149,7 @@ void exec_job(parser_t &parser, job_t *j) {
     j->set_flag(JOB_CONSTRUCTED, true);
     if (!j->get_flag(JOB_FOREGROUND)) {
         proc_last_bg_pid = j->pgid;
+        env_set(L"last_pid", ENV_GLOBAL, { to_string(proc_last_bg_pid) });
     }
 
     if (!exec_error) {

src/expand.cpp

@@ -564,67 +564,6 @@ static void find_process(const wchar_t *proc, expand_flags_t flags,
     }
 }
 
-/// Process id expansion.
-static bool expand_pid(const wcstring &instr_with_sep, expand_flags_t flags,
-                       std::vector<completion_t> *out, parse_error_list_t *errors) {
-    // Hack. If there's no INTERNAL_SEP and no PROCESS_EXPAND, then there's nothing to do. Check out
-    // this "null terminated string."
-    const wchar_t some_chars[] = {INTERNAL_SEPARATOR, PROCESS_EXPAND, L'\0'};
-    if (instr_with_sep.find_first_of(some_chars) == wcstring::npos) {
-        // Nothing to do.
-        append_completion(out, instr_with_sep);
-        return true;
-    }
-
-    // expand_string calls us with internal separators in instr...sigh.
-    wcstring instr = instr_with_sep;
-    remove_internal_separator(&instr, false);
-
-    if (instr.empty() || instr.at(0) != PROCESS_EXPAND) {
-        // Not a process expansion.
-        append_completion(out, instr);
-        return true;
-    }
-
-    const wchar_t *const in = instr.c_str();
-
-    // We know we are a process expansion now.
-    assert(in[0] == PROCESS_EXPAND);
-
-    if (flags & EXPAND_FOR_COMPLETIONS) {
-        if (wcsncmp(in + 1, SELF_STR, wcslen(in + 1)) == 0) {
-            append_completion(out, &SELF_STR[wcslen(in + 1)], COMPLETE_SELF_DESC, 0);
-        } else if (wcsncmp(in + 1, LAST_STR, wcslen(in + 1)) == 0) {
-            append_completion(out, &LAST_STR[wcslen(in + 1)], COMPLETE_LAST_DESC, 0);
-        }
-    } else {
-        if (wcscmp((in + 1), SELF_STR) == 0) {
-            append_completion(out, to_string<long>(getpid()));
-            return true;
-        }
-        if (wcscmp((in + 1), LAST_STR) == 0) {
-            if (proc_last_bg_pid > 0) {
-                append_completion(out, to_string<long>(proc_last_bg_pid));
-            }
-            return true;
-        }
-    }
-
-    // This is sort of crummy - find_process doesn't return any indication of success, so instead we
-    // check to see if it inserted any completions.
-    const size_t prev_count = out->size();
-    find_process(in + 1, flags, out);
-
-    if (prev_count == out->size() && !(flags & EXPAND_FOR_COMPLETIONS)) {
-        // We failed to find anything.
-        append_syntax_error(errors, 1, FAILED_EXPANSION_PROCESS_ERR_MSG,
-                            escape_string(in + 1, ESCAPE_NO_QUOTED).c_str());
-        return false;
-    }
-
-    return true;
-}
-
 /// Parse an array slicing specification Returns 0 on success. If a parse error occurs, returns the
 /// index of the bad token. Note that 0 can never be a bad index because the string always starts
 /// with [.
@@ -1340,7 +1279,7 @@ static expand_error_t expand_stage_brackets(const wcstring &input, std::vector<c
     return expand_brackets(input, flags, out, errors);
 }
 
-static expand_error_t expand_stage_home_and_pid(const wcstring &input,
+static expand_error_t expand_stage_home(const wcstring &input,
                                                 std::vector<completion_t> *out,
                                                 expand_flags_t flags, parse_error_list_t *errors) {
     wcstring next = input;
@@ -1348,16 +1287,7 @@ static expand_error_t expand_stage_home_and_pid(const wcstring &input,
     if (!(EXPAND_SKIP_HOME_DIRECTORIES & flags)) {
         expand_home_directory(next);
     }
-
-    if (flags & EXPAND_FOR_COMPLETIONS) {
-        if (!next.empty() && next.at(0) == PROCESS_EXPAND) {
-            expand_pid(next, flags, out, NULL);
-            return EXPAND_OK;
-        }
-        append_completion(out, next);
-    } else if (!expand_pid(next, flags, out, errors)) {
-        return EXPAND_ERROR;
-    }
+    append_completion(out, next);
     return EXPAND_OK;
 }
 
@@ -1463,7 +1393,7 @@ expand_error_t expand_string(const wcstring &input, std::vector<completion_t> *o
 
     // Our expansion stages.
     const expand_stage_t stages[] = {expand_stage_cmdsubst, expand_stage_variables,
-                                     expand_stage_brackets, expand_stage_home_and_pid,
+                                     expand_stage_brackets, expand_stage_home,
                                      expand_stage_wildcards};
 
     // Load up our single initial completion.

src/expand.h

@@ -60,8 +60,6 @@ class completion_t;
 enum {
     /// Character representing a home directory.
     HOME_DIRECTORY = EXPAND_RESERVED_BASE,
-    /// Character representing process expansion.
-    PROCESS_EXPAND,
     /// Character representing variable expansion.
     VARIABLE_EXPAND,
     /// Character representing variable expansion into a single element.

src/fish_tests.cpp

@@ -4422,7 +4422,6 @@ static void test_illegal_command_exit_code() {
         {L"echo -n", STATUS_CMD_OK}, {L"pwd", STATUS_CMD_OK},
         {L")", STATUS_ILLEGAL_CMD},  {L") ", STATUS_ILLEGAL_CMD},
         {L"*", STATUS_ILLEGAL_CMD},  {L"**", STATUS_ILLEGAL_CMD},
-        {L"%", STATUS_ILLEGAL_CMD},  {L"%test", STATUS_ILLEGAL_CMD},
         {L"?", STATUS_ILLEGAL_CMD},  {L"abc?def", STATUS_ILLEGAL_CMD},
         {L") ", STATUS_ILLEGAL_CMD}};
 

src/highlight.cpp

@@ -120,7 +120,6 @@ bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_l
     for (size_t i = 0; i < path_with_magic.size(); i++) {
         wchar_t c = path_with_magic.at(i);
         switch (c) {
-            case PROCESS_EXPAND:
             case VARIABLE_EXPAND:
             case VARIABLE_EXPAND_SINGLE:
             case BRACKET_BEGIN:
@@ -535,8 +534,7 @@ static void color_argument_internal(const wcstring &buffstr,
                 } else {
                     // Not a backslash.
                     switch (c) {
-                        case L'~':
-                        case L'%': {
+                        case L'~': {
                             if (in_pos == 0) {
                                 colors[in_pos] = highlight_spec_operator;
                             }

src/parse_constants.h

@@ -259,7 +259,7 @@ void parse_error_offset_source_start(parse_error_list_t *errors, size_t amt);
 #define ERROR_NOT_STATUS _(L"$? is not the exit status. In fish, please use $status.")
 
 /// Error issued on $$.
-#define ERROR_NOT_PID _(L"$$ is not the pid. In fish, please use %%self.")
+#define ERROR_NOT_PID _(L"$$ is not the pid. In fish, please use $pid.")
 
 /// Error issued on $#.
 #define ERROR_NOT_ARGV_COUNT _(L"$# is not supported. In fish, please use 'count $argv'.")