I have about fifty git branches for fish and I almost always `git checkout`
between the most recent two or three - this makes the completions list more
usable. If you're using `git cherry-pick` or `git merge`, etc. you also most
likely to want to reference a recently changed branch.
The decision was made to only sort local branches and not remote ones in the PR
at #9248.
The performance of changing from one `git for-each-ref` invocation to two
separate ones (so we could sort them separately) was checked and found to be OK.
Food for future thought: consider ergonomics, caveats, and performance of
excluding the current branch's name from the list of completions (or perhaps
only from the first completion). Or maybe there's another way to have
`for-each-ref` give priority to a different branch while still sorting by
recency?
There are a million existing ways of skinning this cat, but it's a good parallel
to `__fish_seen_argument` to have, in a similar vein to
`__fish_seen_subcommand_from`.
Confirmed on NetBSD: The `ls -o` option groups. I tested `ls -gon` and
it didn't give an error.
It's quite suspect that this one option couldn't be grouped, so I'm
assuming this was a typo.
Prior to 1811a2d, the return value for negative return codes was UB and I'd
witnessed both expected cases like -256 mapping to a $status of 0 and unexpected
cases like a return value of -1 mapping to a $status of 0. As such, this doesn't
test just one fixed return value but the entire range from negative multiples of
256 all the way down (rather, up!) to -1.
While we hardcode the return values for the rest of our builtins, the `return`
builtin bubbles up whatever the user returned in their fish script, allowing
invalid return values such as negative numbers to make it into our C++ side of
things.
In creating a `proc_status_t` from the return code of a builtin, we invoke
W_EXITCODE() which is a macro that shifts left the return code by some amount,
and left-shifting a negative integer is undefined behavior.
Aside from causing us to land in UB territory, it also can cause some negative
return values to map to a "successful" exit code of 0, which was probably not
the fish script author's intention.
This patch also adds error logging to help catch any inadvertent additions of
cases where a builtin returns a negative value (should one forget that unix
return codes are always positive) and an assertion protecting against UB.
Hyphenation in our documentation is aggressive, even to the point of caus-
ing options themselves to be broken across lines. This makes the document-
ation hard to read, especially when you have an option like `string colle-
ct` which gets a weird hyphen.
Remove the hyphenation from the CSS.
This makes it so we link to the very top of the document instead of a
special anchor we manually include.
So clicking e.g. :doc:`string <cmds/string>` will link you to
cmds/string.html instead of cmds/string.html#cmd-string.
I would love to have a way to say "this document from the root of the
document path", but that doesn't appear to work, I tried
`/cmds/string`.
So we'll just have to use cmds/string in normal documents and plain
`string` from other commands.
This was always the case if HAVE_TEXT wasn't defined, but if it was then we were
coercing the result of `_C()` to a `const wchar_t *` pointer, because we were
returning the address of a constant zero-length wchar_t pointer. This reserves a
local static `wcstring` variable that we can return as the "no text" sentinel
and bubbles back the `wcstring` reference rather than decomposing it into a
pointer.
This is a prerequisite for a bigger change I'm working on.
It's gone from 136 bytes to a 128 bytes by rearranging the items in order of
decreasing alignment requirements. While this reduces the memory consumption
slightly (by around 6%) for each completion we have in-memory, that translates
to only around ~8KiB of savings for a command with 1000 possible completions,
which is nice but ultimately not that big of a deal.
The bigger benefit is that a single `complete_entry_t` might now fit in a cache
line, hopefully making the process of testing completions for matches more
cache friendly (and maybe even faster).
...for improved cross-platform support.
Following up on the work in c90ac7b. There was one more test that had mktemp in
the littlecheck "shebang" and this also removes a now-unnecessary `env` prefix.
Commit 09685c3682 tried making the apt
completions faster by doing two things:
1. Introduce a limiting "head"
2. Re-replace our "string" usage with tr
Unfortunately, in doing so it introduced a few issues:
1. The "tr" had a dangling "+" so it cut apart package
descriptions that contained a "+".
This caused e.g. "a C++ library" to generate another completion
candidate, "library".
2. In reusing "tr" it probably reintroduced #8575,
as tr is not 8-bit-clean.
3. It filtered too early, on the raw apt-cache output,
which caused it to fill up with long descriptions.
So e.g. for "texlive" it would only generate 10 completions,
where it should have matched 54 packages.
Because most of the speedup is in the "head" stopping early, we
instead go back to the old string way, but introduce a limiting "head"
after the "sed" (which will have removed everything but the package
name line and the first line of the description)
In my tests this is about ~10% slower than doing head early and using
tr, but it's more correct.
Admittedly I haven't been able to reproduce the 35s scenario that
09685 talks about, but the most likely cause of that is *apt-cache*
being slow - I don't see how string can be that much slower on another
system - and so it will most likely also be fixed by doing head here.
Future possibilities here include:
1. Using "apt-cache search --names-only", which gives a much nicer
format (but only for non-installed packages - the search strings are
apparently ANDed?)
2. Switching to `string split`, possibly using NUL and using `string
split0`?
3. Introducing a `string --null-in` switch so we can get by with one
`string`
4. (multi-threaded execution so the `string`s run in parallel)
All usages of `mktemp` must go through the (fish-only) `mktemp` test function
that abstracts over the differences across multiple platforms/flavors.
Tests can be easily run individually via `ninja -C build test_xxx` and there
isn't a good reason to randomly manually override $HOME and $XDG_CONFIG_HOME for
a test here and a test there.
If it's absolutely necessary, littlecheck.py should be extended to support a
`%temp` variable initialized to a temporary directory and that can be used
instead of calling out to the platform-provided `mktemp` via a subshell.
The impact here depends on the command and how much output it
produces.
It's possible to get up to 1.5x - `string upper` being a good example,
or a no-op `string match '*'`.
But the more the command actually needs to do, the less of an effect
this has.
