25afc9b377 made this unnecessary by
having child processes wait for a signal after fork(), but this change
was later reverted. If we artificially slow down fish (e.g. with a sleep)
after the fork call, we see commands getting backgrounded by mistake.
Put back the tcsetgrp() call.
* Prevent not-yet-loaded functions from loaded when erased
Today, `functions --erase $function` does nothing if the function
hasn't been autoloaded yet.
E.g. run, in an interactive session
> functions --erase ls
> type ls
and be amazed that it still shows our default `ls --color=auto`
wrapper function.
This seems counter-intuitive - removing a function ought to remove it,
whether it had been executed before or not.
* doc/changelog
Instead of requiring a flag to enable newline trimming, invert it so the
flag (now `--no-trim-newlines`) disables newline trimming. This way our
default behavior matches that of sh's `"$(cmd)"`.
Also change newline trimming to trim all newlines instead of just one,
again to match sh's behavior.
The `string collect` subcommand behaves quite similarly in practice to
`string split0 -m 0` in that it doesn't split its output, but it also
takes an optional `--trim-newline` flag to trim a single trailing
newline off of the output.
See issue #159.
It's always a bit annoying that `*` requires quoting.
So we allow "x" as an alternative, only it needs to be followed by
whitespace to distinguish it from "0x" hexadecimal notation.
To support distinct parsers having different working directories, we need
to keep the working directory alive, and also retain a non-path reference
to it.
Because an exported universal variable must be exported in all variable
stacks, explicit invalidation is infeasible. Switch the universal variables
to a generation count.
Prior to this fix, fish would invalidate the exported variable list
whenever an exported variable changes. However we soon will not have a
single "exported variable list." If a global variable changes, it is
infeasible to find all exported variable lists and invalidate them.
Switch to a new model where we store a list of generation counts. Every
time an exported variable changes, the node gets a new generation. If the
current generation list does not match the cached one, then we know that
our exported variable list is stale.
This was undocumented, not all that useful and potentially unwanted.
In particular it means that things like
mysql -p(read)
will still keep the password in history.
Also it allows us to simply implement asking for the history deletion
term.
See #5791.
This makes the following changes:
1. Events in background threads are executed in those threads, instead of
being silently dropped
2. Blocked events are now per-parser instead of global
3. Events are posted in builtin_set instead of within the environment stack
The last one means that we no longer support event handlers for implicit
sets like (example) argv. Instead only the `set` builtin (and also `cd`)
post variable-change events.
Events from universal variable changes are still not fully rationalized.
This cuts down on the wcs2string here by ~25%.
The better solution would be to cache narrow versions of $PATH, since
we compute that over and over and over and over again, while it rarely changes.
Or we could add a full path-cache (where which command is), but that's
much harder to invalidate.
See #5905.
This sets the explicit path to the default one, which should be okay,
since the default path never changes (not even if $XDG_CONFIG_HOME
does).
Then it saves a narrow version of that, which saves most of the time
needed to `sync` in most cases.
Fixes#5905.
Note that this isn't technically *w*util, but the differences between
the functions are basically just whether they do the wcs2string
themselves or not.
This fixes a race condition in the topic monitor. A thread may decide to
enter the wait queue, but before it does the generation list changes, and
so our thread will wait forever, resulting in a hang.
It also simplifies the implementation of the topic monitor considerably;
on reflection the whole "metagen" thing isn't providing any value and we
should just compare generations directly.
In the new design, we have a lock-protected list of current generations,
along with a boolean as to whether someone is reading from the pipe. The
reader (only one at a time) is responsible for broadcasting notifications
via a condition variable.
The enum starts at 0 (defined to be!), so we can eliminate this one.
That allows us to remove a reliance on the position of
beginning_of_line, and it would trigger a "type-limits" warning.
Also leave a comment because I actually hit that.
Someone has hit the 10MiB limit (and of course it's the number of
javascript packages), and we don't handle it fantastically currently.
And even though you can't pass a variable of that size in one go, it's
plausible that someone might do it in multiple passes.
See #5267.
This widens the remaining ones that don't take a char
anywhere.
The rest either use a char _variable_ or __FUNCTION__, which from my
reading is narrow and needs to be widened manually. I've been unable
to test it, though.
See #5900.
This solves the main part of (careful linebreak)
issue #5900.
I'm betting all the errors that do use narrow IO are broken, including
a bunch of asserts.
This adds a new mechanism for logging, intended to replace debug().
The entry points are FLOG and FLOGF. FLOG can be used to log a sequence of
arguments, FLOGF is for printf-style formatted strings.
Each call to FLOG and FLOGF requires a category. If logging for a category
is not enabled, there is no effect (and arguments are not evaluated).
Categories may be enabled on the command line via the -d option.
Now that our interactive signal handlers are a strict superset of
non-interactive ones, there is no reason to "reset" signals or take action
when becoming non-interactive. Clean up how signal handlers get installed.
The signal handlers for interactive and non-interactive SIGINT were distinct
and talked to the reader. This wasn't really justified and will complicate
having multiple threads. Unify these into a single signal handler.
is_interactive_read is a suspicious flag which prevents a call to
parser_t::skip_all_blocks from a ^C signal handler. However we end
up skipping the blocks later when we exit the read loop.
This flag seems unnecessary. Bravely remove it.
When setting a variable without a specified scope, we should give priority
to an existing local or global above an existing universal variable with
the same name.
In 16fd780484 there was a regression that
made universal variables have priority.
Fixes#5883
Otherwise we'd undo the history search when you press e.g. execute,
which means you'd execute the search term.
Only `cancel` should walk it back, like it previously did hardcoded to
escape.
Fixes#5891.
These were inadvertently disabled by a bug which was introduced in
cd7e8f4103 . Fix the bug so the tests run
again.
They don't all pass yet; they regressed during the period they were
disabled.
This mainly is conceptually a bit simpler. The comment about making it
cheaper is entirely misplaced since this is quite far away from being
important.
Even expanding 1000 abbrs, it doesn't show up in the profile.
This was, under some circumstances, apparently off by one.
If a suggestion was really long, like
```fish
infocmp | string split , | string trim | string match -re . | while read -d = -l key val; test -z "$val"; and continue; string match -q '*%*' -- $val; and continue; test (string replace -ra '\e([\[\]]|\(B).*[\comJKsu]' '' -- a(tput $key)b) = ab; or echo $key $val; end > xterm
```
(I'm assuming longer than $COLUMNS), it would staircase like with a wrong wcwidth.
This reverts commit 15a5c0ed5f.
I noticed my debug output for 24bit color mode was garbled due to
this being wrong. I spent a little time trying to get the compiler
to tell us about these, but -Wformat doesn't do anything for wchar
printf functions, and __attribute__((format(printf, n, m))) will
cause an error with wchar_t's, so I gave up and decided to manually
check out every '%s' in the entire project. I found (only) one
more.
debug(0, "%s", wchars) will report warnings for incorrect
specifiers but debug(0, L"%s", wchars) is unable. Thus there may
be reason to prefer not using L"..." as an argument if all else
is equal and it's not necessary.
Allows `fish_indent -w **.fish` to restyle all fish files under the
current directory.
(This also has the sideeffect of reducing style.fish time by ~10s, as
we only need to invoke `fish_indent` once, instead of once per-file)
Blocks will soon need to be shared across parsers. Migrate the loop status
(like break or continue) from the block into the libdata. It turns out we
only ever need one, we don't need to track this per-block.
Make it an enum class.
Brace expansion with single words in it is quite useless - `HEAD@{0}`
expanding to `HEAD@0` breaks git.
So we complicate the rule slightly - if there is no variable expansion
or "," inside of braces, they are just treated as literal braces.
Note that this is technically backwards-incompatible, because
echo foo{0}
will now print `foo{0}` instead of `foo0`. However that's a
technicality because the braces were literally useless in that case.
Our tests needed to be adjusted, but that's because they are meant to
exercise this in weird ways.
I don't believe this will break any code in practice.
Fixes#5869.
Prior to this fix, a function_block stored a process_t, which was only used
when printing backtraces. Switch this to an array of arguments, and make
various other cleanups around null terminated argument arrays.
We previously checked if fish_mode_prompt existed as a function, but
that's a bad change for those who already set it to an empty function
to have a mode display elsewhere.
Updated widechar_width takes care of it.
Technically, this does ~3 comparisons more per-character (because it
checks variation selectors and such), but that shouldn't really matter.
get_current_winsize() is intended to be lazy. It does the following:
1. Gets the termsize from the kernel
2. Compares it against the current value
3. If changed, sets COLUMNS and LINES variables
Upon setting these variables, we notice that the termsize has changed
and invalidate the termsize. Thus we were doing this work multiple times
on every screen repaint.
Put back an old hack that just marked the termsize as valid at the end
of get_current_winsize().
This just sets some special characters that we use in the reader, so
it only needs to be done before the reader is set up.
Which, as it stands, is in env_init().
This stops trying to see if the previous line is wider if it is a
prefix of the current one.
Which turns out to be true often enough that it's a net benefit.
This passes character width as an argument for a few functions.
In particular, it hardcodes a width of "1" for a space literal.
There's no reason to compute wcwidth for the length of the prompt.
This measured *all* the characters on the commandline, and saved all
of them in another wcstring_list_t, just to then do... nothing with
that info.
Also, it did wcslen for something that we already have as wcstring,
reserved a vector and did a bunch of work for autosuggestions that
isn't necessary if we have more than one line.
Instead, we do what we need, which is to figure out if we are
multiline and how wide the first line is.
Fixes#5866.
line_shared_prefix explains in its comment that
> If the prefix ends on a combining character, do not include the
previous character in the prefix.
But that's not what it does.
Instead, what it appears to do is to return idx for *every* combining
mark. This seems wrong to begin with, and it also requires checking
wcwidth for *every* character.
So instead we don't do that. If we find the mismatch, we check if it's
a combining mark, and then go back to the previous character (i.e. the
one before the one that the combining mark is for).
My tests found no issues with this, other than a 20% reduction in
pasting time.
The old commit #3f820f0 "Disable ONLCR mapping of NL output to CR-NL"
incorrectly used c_iflag instead of c_oflag, and I copied that error
in my patch. Fixed that. However, there seems to be other problems
trying to use "\x1B[A", which I have not tried to debug, so comment that out.
(However, #3f820f0 seems to mostly work if we fix it to use c_oflag.)
This read something like `o=!_validate_int`, and the flag modifier
reading kept the pointer after the `!`, so it created a long flag
called `_validate_int`, which meant it would not only error out form
```fish
argparse 'i=!_validate_int' 'o=!_validate_int' -- $argv
```
with "Long flag '_validate_int' already defined", but also set
$_flag_validate_int.
Fixes#5864.
As mentioned in #2900, something like
```fish
test -n "$var"; and set -l foo $var
```
is sufficiently idiomatic that it should be allowable.
Also fixes some additional weirdness with semicolons.
This runs build_tools/style.fish, which runs clang-format on C++, fish_indent on fish and (new) black on python.
If anything is wrong with the formatting, we should fix the tools, but automated formatting is worth it.
This removes semicolons at the end of the line and collapses
consecutive ones, while replacing meaningful semicolons with newlines.
I.e.
```fish
echo;
```
becomes
```fish
echo
```
but
```fish
echo; echo
```
becomes
```fish
echo
echo
```
Fixes#5859.
This was a sort of side channel that was only used to propagate redraws
after universal variable changes. We can eliminate it and handle these
more directly.
tsan does funny things to signals, preventing signals from being delivered
in a blocking read. Switch the topic monitor to non-blocking reads under
tsan.
This keeps all unknown options in $argv, so
```fish
argparse -i a/alpha -- -a banana -o val -w
```
results in $_flag_a set to banana, and $argv set to `-o val -w`.
This allows users to use multiple argparse passes, or to simply avoid
specifying all options e.g. in completions - `systemctl` has 46 of
them, most not having any effect on the completions.
Fixes#5367.
This cleans up how functions are stored and autoloaded. It eliminates the
recursive lock. Instead there is a single normal owning_lock that protects
the entirety of the function data. Autoloading is re-implemented via the
new autoloader_t.
autoloader_t will be the reimplementation of autoloading. Crucically it no
longer manages any locking or loading itself; instead all locking and loading
is performed by clients. This makes it easier to test and helps limit its
responsibilities.
autoloading has a "feature" where functions are removed in an LRU-fashion.
But there's hardly any benefit in removing autoloaded functions. Just stop
doing it.
This is a long-standing issue with how `complete --do-complete` does
its argument parsing: It takes an optional argument, so it has to be
attached to the token like `complete --do-complete=foo` or (worse)
`complete -Cfoo`.
But since `complete` doesn't take any bare arguments otherwise (it
would error with "too many arguments" if you did `complete -C foo`) we
can just take one free argument as the argument to `--do-complete`.
It's more of a command than an option anyway, since it entirely
changes what the `complete` call _does_.
* Some comment fixes and renaming of is_iterm2_escape_seq.
The comment for is_iterm2_escape_seq incorrectly says "CSI followed by ]".
This is wrong, because CSI is ESC followed by [ (or the seldom-used 0x9b).
The procedure actually matches Operating System Command (OSC) escape codes.
Since there is nothing iterm2-specific about OSC, is_osc_escape_seq
would be a better name.
Also s_desired_append_char documents a non-existent parameter.
* Update broken iterm2 url in comment.
This was added in 04a96f6 but not strictly required to fix#5803
(verified), with the intention of hiding invisible background jobs
(created by invoking a function within a pipeline) from the user, but
that also broke intentionally created jobs from displaying as well.
I'm thinking it can't be done without keeping track of caller context vs
job context.
Closes#5824.
env_scoped_t lives between environment_t and env_stack_t.
It represents the read-only logic of env_stack_t and will be used to back
the new environment snapshot implementation.
These tests used raw, unescaped parentheses to perform `test` logical
grouping, but the test failures weren't caught because the parser
evaluation errors were not being propagated (fixed in bdbd173e).
It was unconditionally returning `parse_execution_success`. This was
causing certain parser errors to incorrectly return after evaluation
with `$status` equal to `0`, as reported after `eval`, `source`, or
sub-`fish` execution.
Prior to this change, fish used a global flag to decide if we should check
for changes to universal variables. This flag was then checked at arbitrary
locations, potentially triggering variable updates and event handlers for
those updates; this was very hard to reason about.
Switch to triggering a universal variable update at a fixed location,
after running an external command. The common case is that the variable
file has not changed, which we can identify with just a stat() call, so
this is pretty cheap.
This reverts commit cdce8511a1.
This change was unsafe. The prior version (now restored) took the lock and
then copied the data. By returning a reference, the caller holds a
reference to data outside of the lock.
This function isn't worth optimizing. Hardly any functions use this
facility, and for those that do, they typically just capture one or two
variables.
* Convert `function_get_inherit_vars()` to return a reference to the
(possibly) existing map, rather than a copy;
* Preallocate and reuse a static (read-only) map for the (very) common
case of no inherited vars;
* Pass references to the inherit vars map around thereafter, never
triggering the map copy (or even move) constructor.
NB: If it turns out the reference is unsafe, we can switch the inherit vars
to be a shared_ptr and return that instead.
I did not realize builtins could safely call into the parser and inject
jobs during execution. This is much cleaner than hacking around the
required shape of a plain_statement.
- fix the carat position expanding e.g. `command $,`
- improve the error reporting for not-allowed command subtitutions
by figuring out where the expansion failed instead of using
SOURCE_LOCATION_UNKNOWN
- allow nullptr for parse_util_licate_brackets_range() out_string
argument if we don't need it to do any work.
Fixes#5812
`eval` has always been implemented as a function, which was always a bit
of a hack that caused some issues such as triggering the creation of a
new scope. This turns `eval` into a decorator.
The scoping issues with eval prevented it from being usable to actually
implement other shell components in fish script, such as the problems
described in #4442, which should now no longer be the case.
Closes#4443.
While `eval` is still a function, this paves the way for changing that
in the future, and lets the proc/exec functions detect when an eval is
used to allow/disallow certain behaviors and optimizations.
This adds an option --print-rusage-self to the fish executable. When set,
this option prints some getrusage stats to the console in a human-readable
way. This will be used by upcoming benchmarking support.
Followup to 394623b.
Doing it in the parser meant only top-level jobs would be reaped after
being `disown`ed, as subjobs aren't directly handled by the parser.
This is also much cleaner, as now job removal is centralized in
`process_clean_after_marking()`.
Closes#5803.
This prevents the `disown` builtin from directly removing jobs out of
the jobs list to prevent sanity issues, as `disown` may be called within
the context of a subjob (e.g. in a function or block) in which case the
parent job might not yet be done with the reference to the child job.
Instead, a flag is set and the parser removes the job from the list only
after the entire execution chain has completed.
Closes#5720.
When popping a scope from the environment stack, we currently do a lot of
nonsense like looking for changed curses variables. We want to centralize
this in env_stack_t so that it can be migrated to the env_dispatch logic.
Move this logic up one level in preparation for doing that.
This new file is supposed to encapsulate all of the logic around
reacting to variable changes, as opposed to the environment core.
This is to help break up the env.cpp monolith.
Prior to this fix, a job would only inherit a pgrp from its parent if the
first command were external. There seems to be no reason for this
restriction and this causes tcsetgrp() churn, potentially cuasing SIGTTIN.
Switch to unconditionally inheriting a pgrp from parents.
This should fix most of #5765, the only remaining question is
tcsetpgrp from builtins.
Prior to this fix, in every call to job_continue, fish would reclaim the
foreground pgrp. This would cause other jobs in the pipeline (which may
have another pgrp) to receive SIGTTIN / SIGTTOU.
Only reclaim the foreground pgrp if it was held at the point of job_continue.
This partially addresses #5765
In tests we would like to arrange for an executable to invoke certain
system calls, e.g. to claim or relinquish control of the terminal. This is
annoying to do portably via e.g. perl. fish_test_helper is a little
program where we can add custom commands to make it act in certain ways.
This set the term modes to the shell-modes, including disabling
ICRNL (translating \cm to \cj) and echo.
The rationale given was that `reader_interactive_init()` would only be
called >= 250ms later, which I _highly_ doubt considering fish's total
startup time is 8ms for me.
The main idea was that this would stop programs like tmuxinator that
send shortcuts early from failing _iff_ the shortcut was \cj, which
also seems quite unusual.
This works both with `rm -i` and `read` in config.fish, because `read`
explicitly calls `reader_push`, which then initializes the shell modes.
The real fix would involve reordering our init so we set up the
modesetting first, but that's quite involved and the remaining issue
should barely happen, while it's fairly common to have issues with a
prompt in config.fish, and the workaround for the former is simpler, so let's leave it for now.
Partially reverts #2578.
Fixes#2980.
Putting larger members before smaller ones will reduce structure
sizes. bools are 1 byte. on 64bit systems I think they reduced:
wgetopt.h:46: 64 to 56 bytes
builtin_history.cpp:30: 48 to 32 bytes
builtin_status.cpp:91: 32 to 24 bytes
tinyexpr.cpp:69: 40 to 32 bytes
The data stored in these containers is small enough that it is worth
creating distinct sets for each lookup.
In a microbenchmark of these changes, the single-lookup version of the
function with lookups gated on the length of input (bypassed entirely if
the input is longer than the longest key in the container) provided a
1.5x-3.5x speedup over the previous implementation.
Additionally, as the collections are static and their contents are never
modified after startup, it makes no sense to continously calculate the
location of and allocate an iterator for the `!= foo.end()` comparison;
the end iterator is now statically cached.
I'm not expecting massive speed gains out of this change, but the parser
does perform enough of these to make it worth optimizing in this way.
This reverts commit 7a74198aa3.
Believe it or not this commit actually increased copying. When accepting
a value you know you're going to take ownership of, just accept it by
value; then temporaries can invoke the move ctor and blah blah blah.
We really need a lightweight refcounted pass-by-value string to make this
less error prone.
If we switch the bind mode, we add a "force-repaint" there just to
redraw the mode indicator.
That's quite wasteful and annoying, considering that sometimes the prompt can take
half a second.
So we add a "repaint-mode" function that just reexecutes the
mode-prompt and uses the cached values for the others.
Fixes#5783.
As it turns out it didn't work much better, and it fell behind in
support when it comes to things that wcwidth traditionally can't
express like variation selectors and hangul combining characters, but
also simply $fish_*_width.
I've had to tell a few people now to rebuild with widecharwidth after
sending them on a fool's errand to set X variable.
So keeping this option is doing our users a disservice.
* Add "expand-abbr" bind function
This can be used to explictly allow expanding abbreviations.
* Make expanding abbr explicit
NOTE: This accepts them for space only, we currently also do it for \n
and \r.
* Remove now dead code
We no longer trigger an abbr implicitly, so we can remove the code
that does it.
* Fix comment
[ci skip]
Directly access the job list without the intermediate job_iterator_t,
and remove functions that are ripe for abuse by modifying a local
enumeration of the same list instead of operating on the iterators
directly (e.g. proc.cpp iterates jobs, and mid-iteration calls
parser::job_remove(j) with the job (and not the iterator to the job),
causing an invisible invalidation of the pre-existing local iterators.
This printed weird things like
```fish
$ functions -x
functions: Unknown option '-x'
(Type 'help functions' for related documentation)
```
Instead, let's make it
```fish
$ functions -x
functions: Unknown option '-x'
(Type 'help functions' for related documentation)
```
This was printed basically everywhere.
The user knows what they executed on standard input.
A good example:
```fish
set c (subme 513)
```
used to print
```
fish: Too much data emitted by command substitution so it was discarded
set -l x (string repeat -n $argv x)
^
in function 'subme'
called on standard input
with parameter list '513'
in command substitution
called on standard input
```
and now it is
```
fish: Too much data emitted by command substitution so it was discarded
set -l x (string repeat -n $argv x)
^
in function 'subme' with arguments '513'
in command substitution
```
See #5434.
Now:
```
cd: Unknown option '-r'
~/dev/fish-shell/share/functions/cd.fish (line 40):
builtin cd $argv
^
in function 'cd' with arguments '-r'
in function 'f'
in function 'd'
in function 'b' with arguments '-1q --wurst'
in function 'a'
called on standard input
```
See #5434.
This printed things like
```
in function 'f'
called on standard input
in function 'd'
called on standard input
in function 'b'
called on standard input
in function 'a'
called on standard input
```
As a first step, it removes the empty lines so it's now
```
in function 'f'
called on standard input
in function 'd'
called on standard input
in function 'b'
called on standard input
in function 'a'
called on standard input
```
See #5434.
This switches env_var_t to be an immutable value type, and stores its
contents via a shared_ptr. This eliminates string copying when fetching
env_var_t values.
If a function process is deferred, allow it to be unbuffered.
This permits certain simple cases where functions are piped to external
commands to execute without buffering.
This is a somewhat-hacky stopgap measure that can't really be extended
to more general concurrent processes. However it is overall an improvement
in user experience that might help flush out some bugs too.
In a job, a deferred process is the last fish internal process which pipes
to an external command. Execute the deferred process last; this will allow
for streaming its output.
I believe this was selected to be artificially low for the sake
of it displaying well in prompts. But people should expect to get
the same output as can be gotten from `hostname`.
Fixes#5758
The code already allowed for variable width (multicell) *display* of the
newline omitted character, but there was no way to define it as being
more than one `wchar_t`.
This lets us use a string on console sessions (^J aka newline feed)
instead of an ambiguous character like `@` (used in some versions of
vim for ^M) or `~` (what we were using).
The system version of `wcwidth()` reflects the capabilities of the
system's own virtual terminal's view of the width of the character in
question, while fish's enhanced version (`widechar_wcwidth`) is much too
smart for most login terminals, which generally barely support anything
beyond ASCII text.
If, at startup, it is detected that we are running under a physical
console rather than within a terminal emulator running in a desktop
environment, take that as a hint to use the system-provided `wcwidth`.
The commit began passing the length of the wide string rather than the
length of the narrowed string after conversion via `wcstombs`. We *do*
have the actual length, but it's not (necessarily) the same as the
original value. We need to pass the result of `wcstombs` instead.
POSIX dictates here that incomplete conversions, like in
printf %d\n 15.2
or
printf %d 14g
are still printed along with any error.
This seems alright, as it allows users to silence stderr to accept incomplete conversions.
This commit implements it, but what's a bit weird is the ordering between stdout and stderr,
causing the error to be printed _after_, like
15
14
15.1: value not completely converted
14,2: value not completely converted
but that seems like a general issue with how we buffer the streams.
(I know that nonfatal_error is a copy of most of fatal_error - I tried
differently, and va_* is weird)
Fixes#5532.
Before this change, - was sorted with other punctuation before
A-Z. Now, it sorts above the rest of the characters.
This has a practical effect on completions, where when there are
both -s and --long with the same description, the short option
is now before the long option in the pager, which is what is now
selected when navigating `foo -<TAB>`. The long options can be
picked out with `foo --<TAB>`. Before, short options which
duplicated a long option literally could not be selected by
any means from the pager.
Fixes#5634
This tweaks wcsfilecmp such that certain punctuation characters will
come after A-Z.
A big win with `set <TAB>` - the __prefixed fish junk now comes
after the stuff users should care about.
This disables an extra round of escaping in the `string replace -r`
replacement string.
Currently, to add a backslash to an a or b (to "escape" it):
string replace -ra '([ab])' '\\\\\\\$1' a
7 backslashes!
This removes one of the layers, so now 3 or 4 works (each one escaped
for the single-quotes, so pcre receives two, which it reads as one literal):
string replace -ra '([ab])' '\\\\$1' a
This is backwards-incompatible as replacement strings will change
meaning, so we put it behind a feature flag.
The name is kinda crappy, though.
Fixes#5474.
As a simple replacement for `wc -l`.
This counts both lines on stdin _and_ arguments.
So if "file" has three lines, then `count a b c < file` will print 6.
And since it counts newlines, like wc, `echo -n foo | count` prints 0.
Mostly related to usage _(L"foo"), keeping in mind the _
macro does a wcstring().c_str() already.
And a smattering of other trivial micro-optimizations certain
to not help tangibly.
C++11 provides std::min/std::max which we're using all over,
obviating the need for our own templates for this.
util.h now only provides two things: get_time and wcsfilecmp.
This commit removes everything that includes it which doesn't
use either; most because they no longer need mini or maxi from
it but some others were #including it unnecessarily.
Hangul uses three codepoints to combine to one glyph. The first has a
width of 2 (like the final glyph), but the second and third were
assigned a width of 1, which seems to match EastAsianWidth.txt:
> 1160..11FF;N # Lo [160] HANGUL JUNGSEONG FILLER..HANGUL JONGSEONG SSANGNIEUN
Instead, we override that and treat the middle and end codepoint as combiners,
always, because there's no way to figure out what the terminal will
think and that's the way it's supposed to work.
If they stand by themselves or in another combination, they'll indeed
show up with a width of 1 so we'll get it wrong, but that's less
likely and not expressible with wcwidth().
Fixes#5729.
This only did prefix matching, which is generally less useful.
All existing users _should_ be okay with this since they want to
provide completions.
Fixes#5467.
Fixes#2318.
This addresses a few places where -Wswitch-enum showed one or two missing
case's for enum values.
It did uncover and fix one apparent oversight:
$ function asd -p 100
echo foo
end
$ functions --handlers-type exit
Event exit
asd
It looks like this should be showing a PID before 'asd' just like
job_exit handlers show the job id. It was falling
through to default: which just printed the function name.
$ functions --handlers-type exit
Event exit
100 asd
This tried to skip conversion if the locale had MB_CUR_MAX == 1, but
in doing so it just entered an infinite recursion (because
writestr(wchar_t*) called writestr(wchar_t*)).
Instead, just let wcstombs handle it.
Fixes#5724.
Since Unicode 9, the width of some characters changed to 2.
Depending on the system, it might have support for it, or it might
not.
Instead of hardcoding specific glibc etc versions, we check what the
system wcwidth says to "😃", U+1F603 "Grinning Face With Big Eyes".
The intention is to, in most cases, make setting $fish_emoji_width
unnecessary, but since it sets the "guessed_emoji_width", that variable still takes precedence if it is set.
Unfortunately this approach has some caveats:
- It relies on the locale being set to a unicode-supporting one.
(C.UTF-8 is unfortunately not standard, so we can't use it)
- It relies on the terminal's wcwidth having unicode9 support IFF the
system wcwidth does.
This is like #5722, but at runtime.
The additional caveat is that we don't try to achieve a unicode
locale, but since we re-run the heuristic when the locale changes (and
we try to get a unicode locale), we should still often get the correct
value.
Plus if you use a C locale and your terminal still displays emoji,
you've misconfigured your system.
Fixes#5722.
Prior to this fix, the wait command used waitpid() directly. Switch it to
calling process_mark_finished_children() along with the rest of the job
machinery. This centralizes the waitpid call to a single location.
It turns out that `string split0` didn't actually ever do any
splitting. The arg_iterator_t already split stdin on NUL, and split0 just
performed an additional search that could never succeed (since
arguments from argv already can't contain NUL).
Let the arg_iterator_t not perform any splitting if asked, and then
let split0 split in 0.
One slight wart is that split0 ignores a trailing NUL, which normal
split doesn't.
Fixes#5701.
This is another case where we used pid when we meant pgroup.
Since 55b3c45f95, the assumption that
both are the same no longer holds in all cases, so this check was wrong.
Might fix#5663.
In fish we play fast and loose with status codes as set directly (e.g. on
failed redirections), vs status codes returned from waitpid(), versus the
value $status. Introduce a new value type proc_status_t to encapsulate
this logic.
prettify_node_recursive is replaced with prettify_node_nrecursive
explicite stack is used instead.
Signed-off-by: Janczar Kurek <janczar.kurek@student.uj.edu.pl>
fish's signal handlers are now sufficiently innocuous that there should
be no reason to block signals (outside of temporarily, when creating a
thread and we need to manipulate the signal mask).
Prior to this fix, an "event" was used as both a predicate on which events
to match, and also as the event itself. Re-express these concepts
distinctly: an event is something that happened, an event_handler is the
predicate and name of the function to execute.
Prior to this fix, fish had a signal_list_t that accumulated signals.
Signals were added to an array of integers, with an overflow flag.
The event machinery would attempt to atomically "swap in" the other list.
After this fix, there is a single list of pending signal events, as an array
of atomic booleans. The signal handler sets the boolean corresponding to its
signal.
In a galaxy far, far away, event_blockage_t was intended to block only cetain
events. But it always just blocked everything. Eliminate the event block
mask.
As it turns out, NetBSD's rand(3) is awful - it's possible that in any
given run it'll only return odd numbers, which means
while (rand() % 10)
will never stop.
Since random(3) is also standardized and works, let's use that!
Now that we use an internal process to perform builtin output, simplify the
logic around how it is performed. In particular we no longer have to be
careful about async-safe functions since we do not fork.
Also fix a bunch of comments that no longer apply.
This uses the new internal process mechanism to write output for builtins.
After this the only reason fish ever forks is to execute external processes.
This introduces "internal processes" which are backed by a pthread instead
of a normal process. Internal processes are reaped using the topic
machinery, plugging in neatly alongside the sigchld topic; this means that
process_mark_finished_children() can wait for internal and external
processes simultaneously.
Initially internal processes replace the forked process that fish uses to
write out the output of blocks and functions.
This adds an "in-process" interpretation of dup2s, allowing for fish to
output directly to the correct file descriptor without having to perform
an in-kernel dup2 sequence.
The sigchld generation expresses the idea that, if we receive a sigchld
signal, the generation will be different than when we last recorded it. A
process cannot exit before it has launched, so check the generation count
before process launch. This is an optimization that reduces failing
waitpid calls.
This is a big change to how process reaping works, reimplenting it using
topics. The idea is to simplify the logic in
process_mark_finished_children around blocking, and also prepare for
"internal processes" which do not correspond to real processes.
Before this change, fish would use waitpid() to wait for a process group,
OR would individually poll processes if the process group leader was
unreapable.
After this change, fish no longer ever calls blocking waitpid(). Instead
fish uses the topic mechanism. For each reapable process, fish checks if
it has received a SIGCHLD since last poll; if not it waits until the next
SIGCHLD, and then polls them all.
topic_monitor allows for querying changes posted to one or more topics,
initially sigchld. This will eventually replace the waitpid logic in
process_mark_finished_children().
Comment from the new header:
Topic monitoring support. Topics are conceptually "a thing that can
happen." For example, delivery of a SIGINT, a child process exits, etc. It
is possible to post to a topic, which means that that thing happened.
Associated with each topic is a current generation, which is a 64 bit
value. When you query a topic, you get back a generation. If on the next
query the generation has increased, then it indicates someone posted to
the topic.
For example, if you are monitoring a child process, you can query the
sigchld topic. If it has increased since your last query, it is possible
that your child process has exited.
Topic postings may be coalesced. That is there may be two posts to a given
topic, yet the generation only increases by 1. The only guarantee is that
after a topic post, the current generation value is larger than any value
previously queried.
Tying this all together is the topic_monitor_t. This provides the current
topic generations, and also provides the ability to perform a blocking
wait for any topic to change in a particular topic set. This is the real
power of topics: you can wait for a sigchld signal OR a thread exit.
This resolves the issue where running pre-compiled Linux packages from
binary package manager repositories lead fish to think that we are not
running under WSL.
- Closes#5619.
- Ping neovim/neovim#7330
This happens on OpenIndiana/Solaris/Illumos/SunOS.
Elsewhere we use read_blocked, which already returned in this
case (and which we might want to use here as well!).
`fish_title` as invoked by fish itself is not running in an interactive
context, and attempts to read from the input fd (e.g. via `read`) cause
fish to segfault, go into an infinite loop, or hang at the read prompt
depending on the exact command line and fish version.
This patch addresses that by explicitly closing the input fd when
invoking `fish_title`.
Reported by @floam in #5629. May close that issue, but situation is
unclear.
Taking advantage of the maybe_t's, the logic and nesting here
can be a bit less intense.
Small adjustments to debug output, and found a more accurate
version number for Lion Terminal.app.
Longer term we should have a terminal_t class or something
encapsulating all the kinds of terminal detection we have
with methods that return the color support, and also stuff
like whether the terminal has the newline glitch, the
ambiguous width character behavior, etc.
fish forks child processes when (for example) writing out builtin output.
After fork it resets signal handlers, but if a signal is delivered before
the signal handlers are reset, it will inherit fish's default handlers,
which do things like swallow SIGINT. Teach fish's default signal handlers
to detect this case and re-raise signals with default handlers.
This improves the reliability of control-C in the face of builtins.
I hope this is now complete.
Also, shorten enough descriptions to make `string match --<TAB>`
show a two column pager with 80 cols.
We really should have shown more retraint in the design of `string`,
not all of the flags required both a long and short option created.
300ms was waaay too long, and even 100ms wasn't necessary.
Emacs' evil mode uses 10ms (0.01s), so let's stay a tad higher in case
some terminals are slow.
If anyone really wants to be able to type alt+h with escape, let them
raise the timeout.
Fixes#3904.
`/tmp` isn't present / writeable on every system. Instead of always
using `/tmp`, try to use standard environment variables and
configuration to find a temporary directory.
Adapted from #3974, with updates based on those comments.
Closes#3845.
This is a large change to how io_buffers are filled. The essential problem
comes about with code like (example):
echo ( /bin/pwd )
The output of /bin/pwd must go to fish, not the tty. To arrange for this,
fish does the following:
1. Invoke pipe() to create a pipe.
2. Add an io_bufferfill_t redirection that owns the write end of the pipe.
3. After fork (or equiv), call dup2() to replace pwd's stdout with this pipe.
Now when /bin/pwd writes, it will send output to the read end of the pipe.
But who reads it?
Prior to this fix, fish would do the following in a loop:
1. select() on the pipe with a 10 msec timeout
2. waitpid(WNOHANG) on the pwd proc
This polling is ugly and confusing and is what is replaced here.
With this new change, fish now reads from the pipe via a background thread:
1. Spawn a background pthread, which select()s on the pipe's read end with
a long (100 msec) timeout.
2. In the foreground, waitpid() (allowing hanging) on the pwd proc.
The big win here is a major simplification of job_t::continue_job() since
it no longer has to worry about filling buffers. This will make things
easier for concurrent execution.
It may not be obvious why the background thread still needs a poll (100 msec).
The answer is for cases where the write end of the fd escapes, in particular
background processes invoked inside command substitutions. psub is perhaps
the only important case of this (other shells typically just hang here).
This makes some significant architectual improvements to io_pipe_t and
io_buffer_t.
Prior to this fix, io_buffer_t subclassed io_pipe_t. io_buffer_t is now
replaced with a class io_bufferfill_t, which does not subclass pipe.
io_pipe_t no longer remembers both fds. Instead it has an autoclose_fd_t,
so that the file descriptor ownership is clear.
This switches IO redirections after fork() to use the dup2_list_t,
instead of io_chain_t. This results in simpler code with much simpler
error handling.
This represents a "resolved" io_chain_t, where all of the different io_data_t
types have been reduced to a sequence of dup2() and close(). This will
eliminate a lot of the logic duplication around posix_spawn vs fork, and pave
the way for in-process redirections.
This is a large change to how io_buffers are filled. The essential problem
comes about with code like (example):
echo ( /bin/pwd )
The output of /bin/pwd must go to fish, not the tty. To arrange for this,
fish does the following:
1. Invoke pipe() to create a pipe.
2. Add an io_bufferfill_t redirection that owns the write end of the pipe.
3. After fork (or equiv), call dup2() to replace pwd's stdout with this pipe.
Now when /bin/pwd writes, it will send output to the read end of the pipe.
But who reads it?
Prior to this fix, fish would do the following in a loop:
1. select() on the pipe with a 10 msec timeout
2. waitpid(WNOHANG) on the pwd proc
This polling is ugly and confusing and is what is replaced here.
With this new change, fish now reads from the pipe via a background thread:
1. Spawn a background pthread, which select()s on the pipe's read end with
a long (100 msec) timeout.
2. In the foreground, waitpid() (allowing hanging) on the pwd proc.
The big win here is a major simplification of job_t::continue_job() since
it no longer has to worry about filling buffers. This will make things
easier for concurrent execution.
It may not be obvious why the background thread still needs a poll (100 msec).
The answer is for cases where the write end of the fd escapes, in particular
background processes invoked inside command substitutions. psub is perhaps
the only important case of this (other shells typically just hang here).
This makes some significant architectual improvements to io_pipe_t and
io_buffer_t.
Prior to this fix, io_buffer_t subclassed io_pipe_t. io_buffer_t is now
replaced with a class io_bufferfill_t, which does not subclass pipe.
io_pipe_t no longer remembers both fds. Instead it has an autoclose_fd_t,
so that the file descriptor ownership is clear.
This switches IO redirections after fork() to use the dup2_list_t,
instead of io_chain_t. This results in simpler code with much simpler
error handling.
This represents a "resolved" io_chain_t, where all of the different io_data_t
types have been reduced to a sequence of dup2() and close(). This will
eliminate a lot of the logic duplication around posix_spawn vs fork, and pave
the way for in-process redirections.
By exclusively waiting by pgrp, we can fail to reap processes that
change their own pgrp then either crash or close their fds. If we wind
up in a situation where `waitpid(2)` returns 0 or ECHLD even though we
did not specify `WNOHANG` but we still have unreaped child processes,
wait on them by pid.
Closes#5596.
If we read an R_EOF, we'd try to match mappings to it.
In emacs mode, that's not an issue because the generic binding was
always available, but in vi-normal mode there is no generic binding,
so we'd endlessly loop, waiting for another character.
Fixes#5528.
Originally I sought out to configure the foreground color of the
selected text in the pager. After reading a thread on a github issue I
was inpired to do more: now you can conifgure any part of the pager when
selected, and when a row is secondary. More specifically this commit adds the
ability to specify a pager row's:
- Prefix
- Completion text
- Description
- Background
when said row is selected or secondary.
This will print out along with the stuff we've guessed about color
support. We get a lot of bug reports about these messing up rendering,
this is useful diagnostic output.
Ask the system where utilities are available with confstr (POSIX).
This is the same string printed by `getconf PATH`, which likely
includes more directories.
I was surprised to see:
> set_color normal | string escape
\e\[30m\e\(B\e\[m
I only expected to see a sgr0 here.
Cleanup a nearby `else { if (...) {` and comment with a bogus example.
There was a bogus check for is_interactive_session. But if we are in
reader_readline we are necessarily interactive (even if we are not in
an interactive session, i.e. a fish script invoked some interactive
functionality).
Remove this check.
Fixes#5519
A while loop now evaluates to the last executed command in the body, or
zero if the loop body is empty. This matches POSIX semantics.
Add a bunch of tricky tests.
See #4982
This is effectively a pick of 2ebdcf82ee
and the subsequent fixup. However we also avoid setting WNOHANG unless
waitpid() indicates a process was reaped.
Fixes#5438
For some reason, we have two places where a variable can be read-only:
- By key in env.cpp:is_read_only(), which is checked via set*
- By flag on the actual env_var_t, which is checked e.g. in
parse_execution
The latter didn't happen for non-electric variables like hostname,
because they used the default constructor, because they were
constructed via operator[] (or some such C++-iness).
This caused for-loops to crash on an assert if they used a
non-electric read-only var like $hostname or $SHLVL.
Instead, we explicitly set the flag.
We might want to remove one of the two read-only checks, or something?
Fixes#5548.
Our is_hex_digit() was redundant, we can just use iswxdigit; the libc
implementation is a more efficient table lookup anyhow.
Do is_octal_digit() in terms of iswdigit instead of using wcschr.
This requires threading environment_t through many places, such as completions
and history. We introduce null_environment_t for when the environment isn't
important.
`xlocale.h` is not available on Linux, so we can't just universally
include it.
`HAVE_XLOCALE_H` was already being tested/set in the CMake script as a
possible requirement for `wcstod_l` support, this just adds it to
`config_cmake_h.in` and uses it in `wutil.h` to gate the include.
Removes the dependency on the current user's home directory, instead
overriding it to be within the current hierarchy.
Fixes the tests on Debian buildd, where the home directory is
deliberately unwriteable to pick up errors in builds.
Using `setlocale` is both not thread-safe and not correct, as
a) The global locale is usually stored in static storage, so
simultaneous calls to `setlocale` can result in corruption, and
b) `setlocale` changes the locale for the entire application, not
just the calling thread. This means that even if we wrapped the
`wcstod_l` in a mutex to prevent the previous point, the results
would still be incorrect because this would incorrectly influence the
results of locale-aware functions executed in other threads while
this thread is executing.
The previous comment mentioned that `uselocale` hadn't worked. I'm not
sure what the failing implementation looked like, but `uselocale` can be
tricky. The committed implementation passes the tests for me under Linux
and FreeBSD.
This was the actual issue leading to memory corruption under FreeBSD in
issue #5453, worked around by correcting the detection of `wcstod_l` so
that our version of the function is not called at all.
If we are 100% certain that `wcstod_l` does not exist, then then the
existing code is fine. But given that our checks have failed seperately
on two different platforms already (FreeBSD and Cygwin/newlib), it's a
good precaution to take.
This helps on netbsd, because enter_standout_mode et al are const
there.
These methods don't alter their argument, so they should have been
const to begin with.
This is non-const on macOS, but some of the args we pass are always
const on netbsd.
I have no idea why you'd ever want this to modify its argument, but whatever.
This is the more correct fix for #5447, as regardless of which process
in the job (be it the first or the last) finished first, once we have
waited on a process without ~WNOHANG we don't do that for any subsequent
processes in the job.
It is also a waste to call into the kernel to wait for a process we
already know is completed!
@ridiculousfish had introduced this in 3a45cad12e
to work around an issue with Coverity Scan where it couldn't tell the
mutex was correctly locked, but even with the `fish_mutex_t` hack, it
still emits the same warnings, so there's no pointing in keeping it.
This is necessary for the history race condition test to succeed.
(That test is permanently disabled under WSL (as it always fails) so I
didn't catch this on my end.)
Use `pthread_atfork()` to mark child processes as dirty when `fork()` is
invoked rather than needing to call into the kernel each time
`ASSERT_IS_NOT_FORKED_CHILD()` is called.
This makes simple test cases that hit `ASSERT_IS_NOT_FORKED_CHILD()` 1.8x faster.
------------------------
With a7998c4829 reverted but before this optimization:
```
mqudsi@ZBOOK ~/r/fish-shell> hyperfine -S build/fish 'for i in (seq 100000); test 1 = 1; end'
Benchmark #1: for i in (seq 100000); test 1 = 1; end
Time (mean ± σ): 717.8 ms ± 14.9 ms [User: 503.4 ms, System: 216.2 ms]
Range (min … max): 692.3 ms … 740.2 ms
```
With a7998c4829 reverted and with this optimization:
```
mqudsi@ZBOOK ~/r/fish-shell> hyperfine -S build/fish 'for i in (seq 100000); test 1 = 1; end'
Benchmark #1: for i in (seq 100000); test 1 = 1; end
Time (mean ± σ): 397.2 ms ± 22.3 ms [User: 322.1 ms, System: 79.3 ms]
Range (min … max): 376.0 ms … 444.0 ms
```
Without a7998c4829 reverted and with this optimization:
mqudsi@ZBOOK ~/r/fish-shell> hyperfine -S build/fish 'for i in (seq 100000); test 1 = 1; end'
Benchmark #1: for i in (seq 100000); test 1 = 1; end
Time (mean ± σ): 423.4 ms ± 51.6 ms [User: 363.2 ms, System: 61.3 ms]
Range (min … max): 378.4 ms … 541.1 ms
```
By using a user-land thread-local integer and lock-free (at least under
x86/x64) atomics, we can implement a safe `assert_is_main_thread()`
without calling into the kernel. Thread-local variables are part of
C++11.
This is called a lot in some performance-sensitive areas, so it is worth
optimizing.
This fixes#5438 by having fish block while waiting on a foreground job
via its individual processes by enumerating the procs in reverse order,
such that we hang waiting for the last job in the IO chain to terminate,
rather than the first.
If it's a foreground job, it is related to the currently running exec.
This fixes exec in functions, i.e.
function reload
exec fish
end
would previously always ask about the "function reload" job.
Fixes#5449.
Fixesoh-my-fish/oh-my-fish#664.
