Switch from a linear to a binary search when looking for a matching
string in an enum map. Testing shows this is a little more than twice as
fast when searching for keywords in the sixteen entry keyword_map array.
This speedup doesn't matter much when searching for subbcommands but any
slow down in the parser is unacceptable.
I'm going to use the same mechanism elsewhere such as token_type_map
in src/parse_tree.cpp. But this change only affects the recently
introduce subcommand handling for the history and status commands.
Verified on Cygwin on MS Windows 7 when invoked as
`env LANG=zh_CN.GBK@cjknarrow fish`. No regression seen
when run on other systems with UTF-8 locales.
Fixes#3503
The `status` command currently silently allows incompatible flags (i.e.,
subcommands). Too, using flags to specify subcommands misleads the user
into thinking they can specify multiple subcommands.
We recently modified the `history` command to deprecate using flags for
subcommands. This change does the same for the `status` command.
Fixes#3509
Earlier lint cleanups overlooked a couple of modules because on macOS at
the moment oclint ignores them. I noticed this when I ran `make lint-all`
on Ubuntu.
My earlier attempt with commit 851e449 to eliminate all the compiler
warnings about mixing signed and unsigned ints in an expression
introduced a subtle bug. This fixes that mistake.
Fixes#3488
There was one block of code modified by commit 42458ff7 that had
convoluted, inverted, logic. In the process of collapsing nested
"if" blocks the logic was modified to avoid using "!" everywhere the
bool was tested. Unfortunately I neglected to modify two of the
conditions used to set that var to reflect the changed polarity.
The fish_key_reader program was the only user of the
`set_wait_on_escape_ms()` function and that use was removed with commit
0461743. So remove it from the main fish code. This was found by `make
lint`.
Update the CHANGELOG to more accurately reflect what will be included in
the 2.4.0 release vis-a-vis the `history` command behavior.
I noticed that the compiler was emitting some harmless warnings related
to the history changes so deal with those as well.
This modifies the code path for `set PATH` and `set CDPATH` to emit an
easier to understand warning when an entry in those vars is invalid. For
example
$ set PATH $PATH /tmp/arglebargle
set: Warning: $PATH entry "/tmp/arglebargle": No such file or directory
$ mkdir /tmp/d
$ chmod 0 /tmp/d
$ set PATH $PATH /tmp/d
set: Warning: $PATH entry "/tmp/d": Permission denied
$ touch /tmp/x
$ set PATH $PATH /tmp/x
set: Warning: $PATH entry "/tmp/x": Not a directory
Fixes#3450
* Fix building on Android by avoiding getpwent() if missing with autoconf check
The getpwent() function does not link when building for Android,
and user names on that platform are not interesting anyway.
Using a configure check for stat.st_ctime_nsec fixes building on
Android which has that field but does not define STAT_HAVE_NSEC.
Before this change the Android build failed on the st_ctim.tv_nsec
fallback #else clause.
Taking a different approach here. I can't see why we'd only want to
recognize certain colors. Now, we'll just try all the colors fish might
use.
This could probably be optimized now that there are more
than 8 (or 16) colors fish can do.
It is believed there are no longer any platforms we support that do not
support passing NULL as the second argument to realpath(). So rather
than duplicating the logic to get reasonable behavior from our
wrealpath() wrapper simply remove the redundant implementation.
After implementing `builtin fish_realpath` it was noticed that it did
not behave like GNU `realpath` without options. Which is super annoying
since that was the whole point of implementing the command. Major
failure on my part since I wrote the unit tests to match the behavior of
the existing `wrealpath()` function that I simply exposed as a builtin
command. Rather than actually verifying it behaved in a manner
compatible with GNU realpath.
Also, while the decision to call the builtin `fish_realpath` seemed to
make sense at the time of the original commit further reflection has
shown that to be a silly, idiosyncratic, thing to have done. So rename
it to simply `realpath`.
Fixes 3400
It's the ninth color - on virtual consoles this was likely to
try a color that doesn't work because we checked if max_colors >= 8.
Add another way to reach that color on terminals with only 8 colors
by using bold mode to get a bright.
This has potential to fail by simply rendering as black which can cause
it to be invisible on a white-on-black terminal. Not bad as it's just
making this bell/whistle invisible:
We *really* want to set the omitted newline character apart by having
it appear grey. On (FreeBSD consoles, at least) VCs it's not uncommon
for it to render as a "?". It's particularly confusing if it doesn't
render in a darker color as it cannot be discerned from actual program
output.
When performing fuzzy completion, if a directory segment is
valid, then don't consider it for a fuzzy match even if
the literal match produces no results.
Fixes#3211
The use of wcstoimax causes certain out-of-range values
to be silently truncated (e.g. when converted to a pid),
and is incompatible with FreeBSD (see #626)
This reverts commit 6faa2f9866.
The template has different behavior around interpreting
non-decimal sequences. This doesn't seem to have been intended.
This reverts commit f843eb3d31.
Both GNU and BSD have bugs regarding the classification of
non-characters and private use area characters. Provide wrappers around
iswalnum(), iswalpha(), and isgraph() to provide a consistent
experience. We don't bother to autoconf the use of these wrappers for
several reasons. Including the fact that a binary built for one distro
release should behave correctly on another release (e.g., FreeBSD 10
does the right thing while FreeBSD 11 and 12 do not with respect to
iswalnum() of code points in the range 0xFDD0..0xFDFF).
Also move a few functions from common.* to wutil.* because they are wide
char specific and really belong in the latter module.
Fixes#3050
* Adds a template to parse integers easily.
It's not enough to use intmax_t and check for empty strings: there are
limits. Adds a template to make it easy to parse an integer of any type.
Adds a compiler flag to flag existing dangers.
* nix warning, include <limits>, fix namespace error.
on MacOS `xcodebuild -quiet` will flag these intmax_t -> * conversions,
just use that if you want to find them.
This adds a flag to the `history search` command to limit the number of
matching entries to the first "n". The default is unlimited. This is
mostly useful in conjunction with aliases (i.e., functions) that are
intended to report the "n" most recent matching history entries without
piping the result through the user's pager.
Fixes#3244
If one does a make fish; ./fish - don't use the make-installed paths.
Also, remove huge chunk of nearly duplicated code #ifdef'd __APPLE__
for relocatable dirs in fish.app: the directories under Resources
in the bundle followed by the changes I made around here a few months
ago now are not different enough that they require a special case.
This works fine for fish.app.
I was surprised fish_indent was running from /usr/local/bin
instead of the git checkout when I ran ./fish
after building fish there. This was more easily noticable after my last
commit. I added some debug lines which probably fish could have been
doing already when looking into that.
This is a pretty major thing during fish initialization, commit it for
everyone.
This deprecates the use of long options for history sub-commands (e.g.,
`history --delete`) in favor of proper sub-commands (e.g., `history
delete`). It also eliminates the short options for those sub-commands.
Also change option processing to allow options anywhere on the command
line to match how the vast majority of fish builtins handle flags.
Replace --with-time with --show-time.
Fixes#3367
Adds a color reset thing, to ensure fish tries to use hard colors during
testing.
Also, work on a discrepancy (not introduced by my changes, afaik) when
with some combinations of color settings, and usage of --bold, caused super
flakey color paninting in the pager. Downwards movements that trigger
scrolling vs. upwards movement in the pager would only apply bold to
selections when moving upwards. The bold state of the command completions in
the pager was flipping flops on and off, depending on if there is a description
on the preceding line.
Implement a lame fix by reseting the color to normal and applying a
different style on the rightmost ')' which seems to be what was influencing it.
Makes fish use terminfo for coloring the newline glich char.
Fixes various spots throughout fish where broken strtoi checks
were converting empty strings to zero. Zero is not a valid pid and
this was causing breakage as well when input.
Nix fish_wcstoi - wcstoimax does the same thing.
Improve comments and some general cleanup.
If an interactive job is started, and it is reaped within fish's
exit handler, we may attempt to print its status message after
cur_term has been set to NULL. This results in a crash.
This change makes fish only print the status message if cur_term is
not NULL.
Fixes#3222
Update history docs.
Note - the omission of a mention of timezone was intentional. These were recorded as naive timestamps lacking timezone information in the first place.
Improves the grouping of multiline history entries
by sepearating the timestamps and history entires onto seperate lines.
Use wcsftime() Saves us a conversion, might as well.
Implementing the --shadow-builtin flag has proven to be highly controversial.
Revert the introduction of that flag to the `function` command. If someone
shoots themselves in the foot by redefining a builtin as a function that's
their problem and not our responsibility to protect them from doing so.
Fixes#3319
In the C/POSIX locale EOF on the tty wasn't handled correctly due to a change
a few months ago to fix an unrelated problem with that locale. What is
surprising is that the core fish code doesn't explicitly depend on
input_common_readch returning WEOF if a character isn't seen within
`wait_on_escape_ms` after an escape.
Fixes#3214
The `fish_key_reader` program emits an example `bind` command for the sequence
of keystrokes it sees. However, if that sequence includes a space or del
character the example `bind` command includes extraneous commentary that makes
the command invalid.
Fixes#3262
The recent change to reconcile the history builtin command and function
broke an undocumented behavior of `history --delete`. This change
reinstates that behavior. It also adds an explicit `--exact` search mode
for the `--search` and `--delete` subcommands.
Fixes#3270
Commit acfd3801 included a legitimate bug fix and a second change that
didn't correct an actual bug but made the code more fragile. Revert the
second part of that commit (while also suppressing the uninitialized
variable compiler warning that caused the ill-advised change).
Just use static_cast directly instead of inscrutible "shortcut"
macro.
It was not always used and doesn't seem to do much besides scramble
things up; encountering CAST_INIT() in the code seems likely to lead
to head scratching due to the transformation taking place.
It was added to save folks typing the type twice, now with 100
columns available, let's roll that convenience macro back.
sockaddr_dl:
Perform reinterpret_cast<sockaddr_dl> conversion. The cast affected
alignment and looks fishy to a compiler (but it's fine). Ditch
C-style cast and communicate we're doing that on purpose.
Where we already manage to cover an enum entirely in a switch
statement such that default: cannot be reached, help ensure
it stays that way by condemning that route.
Also adjust a 'const' I came across that is ignored.
A user reported that fish was dying from a SIGSEGV when launched by the
sjterm terminal app. This was traced to a bug in sjterm passing an empty
argv array to the shell. Which, while technically legal, is very unusual
and a bad practice.
Fixes#3269
Fish assumed that it could use tparm to emit escapes to set colors
as long as the color was under 16 or max_colors from terminfo was 256::
if (idx < 16 || term256_support_is_native()) {
// Use tparm to emit color escape
writembs(tparm(todo, idx);
If a terminal has max_colors = 8, here is what happenened, except
inside fish:
> env TERM=xterm tput setaf 7 | xxd
00000000: 1b5b 3337 6d .[37m
> env TERM=xterm tput setaf 9 | xxd
00000000: 1b5b 3338 6d .[39m
The first escape is good, that second escape is not valid.
Bright colors should start at \e[90m:
> env TERM=xterm-16color tput setaf 9 | xxd
00000000: 1b5b 3931 6d .[91m
This is what caused "white" not to work in #3176 in Terminal.app, and
obviously isn't good for real low-color terminals either.
So we replace the term256_support_is_native(), which just checked if
max_colors is 256 or not, with a function that takes an argument and
checks terminfo for that to see if tparm can handle it. We only use this
test, because otherwise, tparm should be expected to output garbage:
/// Returns true if we think tparm can handle outputting a color index
static bool term_supports_color_natively(unsigned int c) { return max_colors >= c; }
...
if (term_supports_color_natively(idx) {
And if terminfo can't do it, the "forced" escapes no longer use the fancy
format when handling colors under 16, as this is not going to be compatible with
low color terminals. The code before used:
else {
char buff[16] = "";
snprintf(buff, sizeof buff, "\x1b[%d;5;%dm", is_fg ? 38 : 48, idx);
I added an intermediate format for colors 0-15:
else {
// We are attempting to bypass the term here. Generate the ANSI escape sequence ourself.
char buff[16] = "";
if (idx < 16) {
snprintf(buff, sizeof buff, "\x1b[%dm", ((idx > 7) ? 82 : 30) + idx + !is_fg * 10);
} else {
snprintf(buff, sizeof buff, "\x1b[%d;5;%dm", is_fg ? 38 : 48, idx);
}
Restores harmony to white, brwhite, brblack, black color names.
We don't want "white" to refer to color color #16, but to the
standard color #8. #16 is "brwhite".
Move comments from output.h to output.cpp
Nuke the config.fish set_color hack for linux VTs.
Sync up our various incomplete color lists and fix all color values.
Colors 0-8 are assumed to be brights - e.g. red was FF0000. Perplexing!
Using this table:
<http://www.calmar.ws/vim/256-xterm-24bit-rgb-color-chart.html>
Fixes#3176
Add some debug output like there is for 24bit mode.
I see now there is no need to setup terminal here - we get called early
sometimes for colors to work in config.fish to work but that is not so fatal.
Just check cur_term and trust get called again soon.
This fixes several problems with how the builtin `history` command handles
arguments. It now complains and refuses to do anything if the user specifies
incompatible actions (e.g., `--search` and `--clear`). It also fixes a
regression introduced by previous changes with regard to invocations that
don't explicitly specify `--search` or a search term.
Enhances the history man page to clarify the behavior of various options.
This change is already far larger than I like so unit tests will be added
in a separate commit.
Fixes#3224.
Note: This fixes only a couple problems with the interactive `history
--delete` command in the `history` function. The main problem will be
dealt with via issue #31.
We were effectively inferring 256 color support **only**.
If terminfo reports 256 max_colors for this $TERM but
that is not named xterm or does not contain "256color" in name,
term256_support_is_native()'s result did not affect the recorded
support.
Noticed with Terminal.app set to nsterm, and a newer ncurses
with good terminfo for the terminal on modern OS X:
http://invisible-island.net/ncurses/terminfo.src.html#toc-_Apple__Terminal_app
We don't seem to mention in the documentation that we were forcing
-t for all interactive uses. If we want to do that we should apply
that in the builtin.
history.fish reimplementing every option and doing things kind of
differently is a real pain and it's not clear if the docs are
referring to the or the wrapper script or both.
Attempting to execute something like `exec "$test"` results in a fish internal
token (a Unicode private use char) being printed in the resulting error
message. That's obviously not desirable as well as confusing.
Fixes#3187
Prior to this fix, when performing completions, we would prepend
the wildcard to the resulting files. When doing fuzzy completions,
we would take some wildcard segment, attempt to locate it in the
final completion, and then replace it with our fuzzy-matched directory.
With this fix, we pass along the "resolved so far" path, and prepend
that instead of doing "surgery" on the completion. This simplifies the
logic.
Fixes#3185
Someone running fish in an unusual locale reported that an `assert()` was
firing when they typed `pkill c`. I traced it to two bugs. First, the
__fish_make_completion_signals command was producing a weird result. Second,
the builtin `complete` command wasn't adequately verifying its arguments.
Fixes#3129
Make `fish_indent`, `fish_key_reader` and `fish` recognize and assign
the same meaning to the `-d` and `-D` flags. Also, fix some errors and
stylistic issues in the associated man pages.
Fixes#3191
Remove isatty() check for stdout - this was added for both stdout
and stdin because "there is no reason to do that", but there is one:
Leaves only the bind command printed ot stdout, this allows
for one to do `fish_key_reader > bind_command.fish` to capture the bind
command while seeing the rest of the output.
After the colorized syntax output in type -a foo, "foo is /usr/..."
would also be colored. (or 'test' in fish_indent foo.fish; echo test).
Make fish_indent reset the color when it's done.
I did some research and experiments. For good or bad the `bind` command
requires the use of wide char codepoints (e.g., \u1234) for non-ASCII
chars. So don't force the use of the POSIX locale, but do provide it as
an option for people who want to see the individual bytes rather than a
decoded wide char.
Simplify the format of the information displayed for each character. There
really isn't much point in providing decimal, octal, and hexadecimal. Just
print hex and symbolic representations.
Add an example `bind` command that a user can copy/paste.
Closes#3183
Another developer noticed that redirecting stdin of `fish_key_reader`
results in weird behavior. Which is not at all surprising. So add checks
to ensure stdin and stdout are attached to a tty.
Add some rudimentary unit tests for this program.
A discussion on Gitter proposed allowing the user to signal their desire to
exit fish_key_reader by pressing \cC or \cD twice in a row. This implements
that.
I also decided to refactor how signals are handled. Most notably receiving a
signal will no longer print a diagnostic message unless you've enabled
debugging with `-d2` (or higher level).
In addition to fixing the setting of the locale to C/POSIX this also
corrects several problems introduced by the commits made in the past
couple of days. As a consequence of dealing with all of this I decided
to refactor the code to simplify one of the overly long functions I
introduced in my previous change.
Fixes#3168
There is no conceivable way in which timef()'s invocation of gettimeofday()
can fail where it makes sense to continue running. Yes, one such,
legitimate, failure mode is a 32-bit kernel and the date is greater than
2038-01-19 03:14:07. If you're running a fish binary on such a system
it's time to upgrade. Otherwise, either the hardware or OS is broken.
Fixes#3167.
* Correct notice about ^C
* Move time deltas to end of the line away from the important info on
left.
* Use timef() instead of gettimteofday() ourselves
* Show time in ms (is this even useful in any unit? Maybe testing escape
delays...)
* Make init more similar to other apps.
```
~ $ set -e TERM; fish
Assertion failed: (!is_missing), function c_str, file src/env.cpp, line 690.
fish: 'fish' terminated by signal SIGABRT (Abort)
```
The tty device timestamps on MS Windows aren't usable because they're always
the current time. So fish can't use them to decide if the entire prompt needs
to be repainted.
Fixes#2859
* if (result == ULLONG_MAX) is always false, likely a typo as
result is unsigned long, and the comment says ULONG_MAX.
* use off_t instead of size_t for file size where it can mismatch
st_size's type in stat.h
For example, an argument 12345^ is a real argument, not a redirection
There's no reason to use ^ here instead of >, and it's annoying to git
users.
Fixes#1873
When given no path, the logic was happy to try to use
an unitialized output_location.
$ fish_indent -w < test.fish
Opening "(null)" failed: Bad address
Initialize the string, and repair the logic to catch this case
and report the problem correctly.
Update Xcode project, HeaderDoc comments.
Fix various invalid HeaderDoc comments. Normalize autoload.cpp/autoload.h as an example of something closer to "proper" HeaderDoc formatting.
Have clang/Xcode validate HeaderDoc comments. Remove key_reader.cpp from Xcode project.
Fix test setup bogosities. Specifically, they weren't hermetic with respect to
locale env vars.
Rewrite the handling of locale vars to simplify the code and make it more like
the pattern most programs employ.
Fixes#3110
We need to actually export the curses/terminfo env vars in order for
`setupterm()` to be able to use them. While fixing this I reworked the
fallback logic implemented by @zanchey in response to issue #1060 in
order to simplify the logic and clarify the error messages.
This does not allow someone to change the curses/terminfo env vars after
the first prompt is displayed (you can but it won't affect the current
fish process). It only makes it possible to set `TERM`, `TERMINFO`, and
`TERMINFO_DIRS` in *config.fish* or similar config file and have them be
honored by fish.
The issue here is that when inserting a common prefix for e.g. a
substring match, we increase the amount of available candidates again to
things the user didn't want.
An example is in share/functions - a completion for "inter" would
previously expand to "__fish_" because it matched:
- __fish_config_interactive.fish
- __fish_print_interfaces.fish
- __fish_print_lpr_printers.fish
The completion afterwards would then show 189 possible matches, only
three of which (the above) actually matched the original "inter".
Fixes#3089.
Cppcheck was complaining about the `return val.c_str()` at the end of the
`wgettext()` function. That would normally a bug since the lifetime of
`val` ends when the function returns. In this particular case that's not
true because the string is interned in a cache. Nonetheless, rather than
suppress the lint warning I decided to modify the API to be more idiomatic.
In the process of fixing the aforementioned lint warning I fixed several other
lint errors in that module.
This required making our copy of `wgetopt()` compatible with the rest of
the fish code. Specifically, by removing its local definitions of the
"_" macro so it uses the same macro used everywhere else in the fish
code. The sooner we kill the use of wide chars the better.
This only eliminates errors reported by `make lint`. It shouldn't cause any
functional changes.
This change does remove several functions that are unused. It also removes the
`desc_arr` variable which is both unused and out of date with reality.
This makes the wide char tests run by `./fish_tests` pass on systems where
sizeof wchar_t is two (e.g., Cygwin). In doing so it corrects several
problems with the underlying code in module *utf8.cpp* such as allowing
five and six byte UTF-8 sequences. They were allowed by the original
Unicode proposal but are not allowed by the adopted standard.
Configure the tty driver to ignore the lnext (\cV) and werase (\cW) characters
so they can be bound to fish functions.
Correct the `fish_key_bindings` program to initialize the tty in the same
manner as the `fish` program.
Fixes#3064
Overwriting the user's clipboard by default is annoying and contributors
don't use it.
This is better served via an explicit binding that calls e.g. `xsel`.
This change allows the user to specify the script name on the CLI in addition
to being redirected from stdin. It also adds a `-w` flag to write the modified
script to the original file.
I noticed that the `test_convert()` function was randomly failing when
run on OS X Snow Leopard. I tracked it down to the `mbrtowc()` function on
that OS being broken. Explicitly testing for UTF-8 prefixes that identify
a sequence longer than four bytes (which the Unicode standard made illegal
long ago) keeps us from having encoding errors on those OS's.
This also makes the errors reported by the `test_convert()` function actually
useful and readable.
Lastly, it makes it possible to build fish on OS X Snow Leopard.
Drops configure check for wcsdup, wcslen, wcscasecmp, wcsncasecmp,
wcwidth, wcswidth, wcstok, fputwc, fgetwc, and wcstol. Drop the fallback
implementations of these on non-Snow Leopard platforms.
Work on #2999.
fwprintf would segfault on DragonFly BSD 1.4.0, released in January
2006. This was fixed by DragonFly BSD 1.4.4, released in April 2006. It
seems unlikely that anyone is still running a ten-year-old, unsupported
version, and hoping that fish will continue to build.
I've checked this in virtual machines.
Work on #2999.
The autoconf-generated config.h contains a number of directives which
may alter the behaviour of system headers on certain platforms. Always
include it in every C++ file as the first include.
Closes#2993.
This change does several things. First, and most important, it allows
dumping the "n" most recent stack frames on each debug() call. Second,
it demangles the C++ symbols. Third, it prepends each debug() message
with the debug level.
Unrelated to the above I've replaced all `assert(!is_forked_child());`
statements with `ASSERT_IS_NOT_FORKED_CHILD()` for consistency.
Fix a minor bogosity I noticed while building fish on OS X Snow
Leopard. It's technically not a bug because only old compilers complain
about the original statement but this change makes the one line this
changes consistent with the rest of the fish code.
It's currently too easy for someone to bork their shell by doing something
like `function test; return 0; end`. That's obviously a silly, contrived,
example but the point is that novice users who learn about functions are
prone to do something like that without realizing it will bork the shell. Even
expert users who know about the `test` builtin might forget that, say, `pwd`
is a builtin.
This change adds a `--shadow-builtin` flag that must be specified to
indicate you know what you're doing.
Fixes#3000
I'm doing this as part of fixing issue #2980. The code for managing tty modes
and job control is a horrible mess. This is a very tiny step towards improving
the situation.
The original `key_reader` program was useful but didn't do much that `xxd`
or `od -tx1z` didn't do. Furthermore, it wasn't built and installed by
default. This change adds features that make it superior to those programs
for decoding interactive key presses and makes it a first-class citizen
like the `fish_indent` program that is always available.
Fixes#2991
The fork (create new process) related debugging messages rely on an
undocumented env var and use `printf()` rather than `debug()`. There are
also errors in how the fork count is tracked that this fixes.
Fixes#2995
Some `oclint` errors regarding "useless parentheses" are meaningfull. But
the vast majority are bogus in as much as removing the parentheses reduces
readability. So fix a few of the egregious uses and otherwise suppress
that error.
I missed restyling a few "switch" blocks to make them consistent with the rest
of the code base. This fixes that oversight. This should be the final step in
restyling the C++ code to have a consistent style. This also includes a few
trivial cleanups elsewhere.
I also missed restyling the "complete" module when working my way from a to z
so this final change includes restyling that module.
Total lint errors decreased 36%. Cppcheck errors went from 47 to 24. Oclint P2
errors went from 819 to 778. Oclint P3 errors went from 3252 to 1842.
Resolves#2902.
For this change I decided to bundle the remaining modules that need to be
resytyled because only two were large enough to warrant doing on their own.
Reduces lint errors from 225 to 162 (-28%). Line count from 3073 to 2465 (-20%).
Another step in resolving issue #2902.
Reduces lint errors from 36 to 33 (-8%). Line count from 1910 to 1476 (-23%).
Another step in resolving issue #2902.
This also fixes a stupid mistake from an earlier commit where I didn't realize
that osx/config.h was meant to be included as a semi-static file in the
repository.
fish_title currently outputs some escaped text, which can confuse
the line driver (#2453). Issue a carriage return so the line driver
knows we are at the beginning of the line, unless we are writing
the title as part of the prompt. In that case, we may have text from
the previous command still on the line and we don't want to move the
cursor.
Fixes#2453
Don't `#include "*.cpp"` modules in other cpp modules. I already took care
of all the builtin_*.cpp modules in my previous change where I restyled
the builtin code. This change fixes the two remaining instances of this
anti-pattern.
Now that the IWYU cleanup has been merged compile all, not just a couple, of
the builtin modules independent of builtin.cpp. That is, no longer `#include
builtin_NAME.cpp` in builtin.cpp. This is more consistent, more in line with
what developers expect, and is likely to reduce mistakes.
Reduces lint errors from 384 to 336 (-13%). Line count from 6307 to 4988 (-21%).
Another step in resolving issue #2902.
- Set PCRE2_SUBSTITUTE_OVERFLOW_LENGTH to get the required buffer length
from pcre2 instead of guessing
- Set PCRE2_SUBSTITUTE_EXTENDED to enable extra goodies in the
replacement string
Remove the "make iwyu" build target. Move the functionality into the
recently introduced lint.fish script. Fix a lot, but not all, of the
include-what-you-use errors. Specifically, it fixes all of the IWYU errors
on my OS X server but only removes some of them on my Ubuntu 14.04 server.
Fixes#2957
The OS X Xcode IDE has a weird requirement that block comments preceding a
function or class definition must begin with three slashes rather than two if
you want the comment displayed in the "Quick Help" window.
Make the history code conform to the new style guide. Every change was
produced by clang-format (e.g., `make style`) with the exception of
comments which were manually reformatted. That has to be done by hand
since clang-format leaves comments alone other than to reflow comment
lines to get them below the allowed line length.
The total number of lines is reduced by 313 lines (13%) in the two
affected files. Line count is generally a poor metric but in this
case it reflects an increase in information density without a loss in
readability. Furthermore, the standardization of braces, whitespace,
and comment style will make it easier for people to read the code.
This reduces the number of warnings by `make lint` from 168 to 87 (a 48%
decrease). Making it much easier to focus on the substantive lint issues.
Further improvements are possible. For example, many comments are not
very helpful (e.g., they point out the obvious) or provide insufficient
detail. But those are beyond the scope of this change.
This is the first step in resolving issue #2902.
The readlink() function does not null terminate the path it returns.
Remove the OS X code that deals with a path buffer that is too short. For
one thing a loop isn't needed since we're told how big of a buffer
is required if the first _NSGetExecutablePath() call fails. But more
important it is so unlikely that the path will be longer than PATH_MAX
that if it is we should just give up.
Fixes 2931.
I just noticed that depending on the state of your working tree there can be
one or more spaces after the modification token and the file name. If there is
more than one space that causes the `string split` to produce unexpected
output.
In keeping with the change made by @ridiculousfish earlier today modify
the `keyword_description()` function to return a const wchar_t pointer.
Also, simplify the `token_type_description()` function to use the recently
introduced mapping array. This changes the wording of many of the token
type descriptions. However, I can't see this as being a problem since
the original descriptions (e.g., "token_redirection") are no clearer to
someone not acquainted with the implementation.
Fish keywords can be quoted and split across lines. Prior to this change
`fish_indent` would retain such odd, obfuscated, formatting. This change
results in all keywords being converted to their canonical form.
This required fixing a bug: the keyword member of parse_node_t wasn't being
populated. This hadn't been noticed prior to now because it wasn't used.
Fixes#2921
This code represents only risk and does nothing useful for anything
that can compile fish.
In C++ situations where __STDC_VERSION__ is unset (as it should be),
fish was assuming we are on < C99 and setting it to __FUNCTION__.
Basically always, __PRETTY_FUNCTION__ ends up reaplaced by __FUNCTION__, this hurt
error message usefulness and richness.
__PRETTY_FUNCTION__: const thing::sub(int)
__FUNCTION__: sub
Prior to this fix, when completing a command that doesn't have a /, we
would prepend each component of PATH and then expand the whole thing. So
any special characters in the PATH would be interpreted when performing
tab completion.
With this fix, we pull the PATH resolution out of complete.cpp and
migrate it to expand.cpp. This unifies nicely with the CDPATH resolution
already in that file. This requires introducing a new expand flag
EXPAND_SPECIAL_FOR_COMMAND, which is analogous to EXPAND_SPECIAL_CD
(which is renamed to EXPAND_SPECIAL_FOR_CD). This flag tells expand to
resolve paths against PATH instead of the working directory.
Fixes#952
When determining the old path, get the existing value in any scope,
not just the set scope. Also only complain about absolute paths:
relative paths are expected to be invalid sometimes.
Modify `fish_indent` to emit redirections without a space before the target of
the redirection; e.g., "2>&1" rather than "2>& 1" as the former is clearer to
humans.
Fixes#2899
Per discussion in pull-request #2891, it's not available on Linux (we just
fill it with zero), and unless run as root on OS X (or other BSD system) it
will be zero. Remove it from file_id_t. Also fix the initialization of the
file_id_t structure.
Fixes#2891
This is a quick and dirty conversion of the atypical, and undocumented,
logging done by env_universal_common.cpp to the usual `debug()` pattern. I
didn't want to drop the messages because they could be useful when
debugging future issues. So I simply converted them to the lowest debug
level using the normal debug() function.
Fixes#2887
Cppcheck has identified a lot of unused functions. This removes funcs that
are unlikely to ever be used. Others that might be useful for debugging I've
commented out with "#if 0".
This fixes all memory leaks found by compiling with
clang++ -g -fsanitize=address and running the tests.
Method:
Ensure that memory is freed by the destructor of its respective container,
either by storing objects directly instead of by pointer, or implementing
the required destructor.
When explicitly asking for the fish version string the information
should go to stdout rather than stderr. Also, there is no reason to use
exit_without_destructors() rather than exit() in that code path. We
actually want the side-effects of exit() such as flushing stdout and
there aren't any threads or other things that could cause a normal exit
to fail when that function is run.
The early return skipped all cleanup.
This problem is a case for the classic "goto fail" paradigm, but this
change instead makes a few adjustments to take advantage of a previously
unused level of indentation to conditionally execute the success path.
The error message now prints the filename instead of "open",
which should be more idiomatic.
Tip:
This patch makes sense if viewed with `git show --ignore-space-change`.
The swap-selection-start-stop function goes to the other end of the highlighted text, the equivalent of `o' for vim visual mode.
Add binding to the swap-selection-start-stop function, `o' when in visual
mode.
Document swap-selection-start-stop, begin-selection, end-selection, kill-selection.
The relevant standards allow the mbtowc/mbrtowc functions to reject
non-ASCII characters (i.e., chars with the high bit set) when the locale
is C or POSIX. The BSD libraries (e.g., on OS X) don't do this but
the GNU libraries (e.g., on Linux) do. Like most programs we need the
C/POSIX locales to allow arbitrary bytes. So explicitly check if we're
in a single-byte locale (which would also include ISO-8859 variants)
and simply pass-thru the chars without encoding or decoding.
Fixes#2802.
The u_int typedef fails to compile on all platforms (e.g. Windows). It
is part of the code imported from tmux.
Update it to the SUS-standard uid_t.
Closes#2821.
Address the feedback from the prior commit:
- Change the sense of return value testing to match more common
comparison idiom
- Test result of fchmod as well as fchown
- Change sense of return value testing around wrename as well
- Include errno where possible in error message
The function fchown is annotated with warn_unused_result. As
formerly used in the code, it would emit a compiler warning
```warning: ignoring return value of ‘fchown’, declared with
attribute warn_unused_result [-Wunused-result]```
This commit notes the return value and emits appropriate error/logging
messages if the call fails, creating more traceable results and
satisfying the compiler.
There is no longer a good reason to detect whether or not getopt_long()
is available. All UNIX implementations we're likely to run on have it. And
if we ever find one that doesn't the right thing to do is not fallback to
getopt() but to include the getopt_long() source in our package like we
do with the pcre2 library. Since it's licensed under LGPL we can legally
do so if it becomes necessary.
This partially addresses issue #2790.
Previously, when decoding UTF-8, we would first run through the
array to compute the correct size, then allocate a buffer of that size,
then run through the array again to fill the buffer, and then copy it
into a std::wstring. With this fix we can copy it into the string
directly, reducing allocations and only requiring a single pass.
This narrows the range of Unicode codepoints fish reserves for its own
use from U+E000 thru U+F8FE (6399 codepoints) to U+F600 thru U+F73F (320
codepoints). This is still not ideal since fish shouldn't be using any
Unicode private-use codepoints but it's a step in the right direction.
This partially addresses issue #2684.
This was used to cache a narrow string representation
of commands, so that if certain system calls returned errors
after fork, we could output error messages without allocating
memory. But in practice these errors are very uncommon, as are
commands that have wide characters. It is simpler to do a best-effort
output of the wide string, instead of caching a narrow string
unconditionally.
Prior to this fix, read_ni would use parse_util_detect_errors
to lint the script to run, and then parser_t::eval() to execute it.
Both functions would parse the script into a parse tree. This allows
us to re-use the parse tree, improving perfomance.
Introduces a new template moved_ref which is like an rvalue reference.
This allows passing around objects while being explicit that the
receiver may acquire ownership. This will help reduce some allocations.
pcre2_substitute() now sets the output buffer length to PCRE2_UNSET (~0)
if the output buffer is determined to be too small. This change keeps
track of the buffer size separately where pcre2 can't touch it.
A better fix would be to let pcre2 tell fish what size buffer it needs.
This can be done with PCRE2_SUBSTITUTE_OVERFLOW_LENGTH, but this
requires pcre2 10.21 or later (released January 12), which may be too
new to introduce as a dependency at this point.
Fixes#2743
