From 2535cf51dba8bebb386e66360f6024dfb311b455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 25 Jun 2021 08:44:41 +0200 Subject: [PATCH 01/17] =?UTF-8?q?changelog:=20add=20new=20=E2=80=98unrelea?= =?UTF-8?q?sed=E2=80=99=20section?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4a1152c..01cf1fdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +* [Unreleased](#unreleased) * [1.8.0](#1-8-0) * [1.7.2](#1-7-2) * [1.7.1](#1-7-1) @@ -26,6 +27,16 @@ * [1.2.0](#1-2-0) +## Unreleased +### Added +### Changed +### Deprecated +### Removed +### Fixed +### Security +### Contributors + + ## 1.8.0 ### Grapheme shaping From 28730438650e510105a4bff00e24552dac295e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 25 Jun 2021 10:23:43 +0200 Subject: [PATCH 02/17] doc: benchmarks: update desktop results with 1.8.0 --- doc/benchmark-results-desktop.svg | 550 ++++++++++++++++++++++++++++++ doc/benchmark.md | 35 +- 2 files changed, 566 insertions(+), 19 deletions(-) create mode 100644 doc/benchmark-results-desktop.svg diff --git a/doc/benchmark-results-desktop.svg b/doc/benchmark-results-desktop.svg new file mode 100644 index 00000000..aac846c8 --- /dev/null +++ b/doc/benchmark-results-desktop.svg @@ -0,0 +1,550 @@ + + + +Gnuplot +Produced by GNUPLOT 5.4 patchlevel 2 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0 + + + + + 200 + + + + + 400 + + + + + 600 + + + + + 800 + + + + + 1000 + + + + + 1200 + + + + + 1400 + + + + + cursor_motion + + + + + dense_cells + + + + + light_cells + + + + + scrolling + + + + + scrolling_bottom_region + + + + + scrolling_bottom_small_region + + + + + scrolling_fullscreen + + + + + scrolling_top_region + + + + + scrolling_top_small_region + + + + + unicode + + + + + + + + + milliseconds (lower is better) + + + + + gnuplot_plot_1 + + + foot-pgo + + + + + + + + + + gnuplot_plot_2 + + + + + + + gnuplot_plot_3 + + + + + + + gnuplot_plot_4 + + + + + + + gnuplot_plot_5 + + + + + + + gnuplot_plot_6 + + + + + + + gnuplot_plot_7 + + + + + + + gnuplot_plot_8 + + + + + + + gnuplot_plot_9 + + + + + + + gnuplot_plot_10 + + + + + + + gnuplot_plot_11 + + + foot-no-pgo + + + + + + + + + + gnuplot_plot_12 + + + + + + + gnuplot_plot_13 + + + + + + + gnuplot_plot_14 + + + + + + + gnuplot_plot_15 + + + + + + + gnuplot_plot_16 + + + + + + + gnuplot_plot_17 + + + + + + + gnuplot_plot_18 + + + + + + + gnuplot_plot_19 + + + + + + + gnuplot_plot_20 + + + + + + + gnuplot_plot_21 + + + alacritty + + + + + + + + + + gnuplot_plot_22 + + + + + + + gnuplot_plot_23 + + + + + + + gnuplot_plot_24 + + + + + + + gnuplot_plot_25 + + + + + + + gnuplot_plot_26 + + + + + + + gnuplot_plot_27 + + + + + + + gnuplot_plot_28 + + + + + + + gnuplot_plot_29 + + + + + + + gnuplot_plot_30 + + + + + + + gnuplot_plot_31 + + + urxvt + + + + + + + + + + gnuplot_plot_32 + + + + gnuplot_plot_33 + + + + + + + gnuplot_plot_34 + + + + + + + gnuplot_plot_35 + + + + + + + gnuplot_plot_36 + + + + + + + gnuplot_plot_37 + + + + + + + gnuplot_plot_38 + + + + + + + gnuplot_plot_39 + + + + + + + gnuplot_plot_40 + + + + + + + gnuplot_plot_41 + + + xterm + + + + + + + + + + gnuplot_plot_42 + + + + gnuplot_plot_43 + + + + + + + gnuplot_plot_44 + + + + gnuplot_plot_45 + + + + gnuplot_plot_46 + + + + gnuplot_plot_47 + + + + + + + gnuplot_plot_48 + + + + gnuplot_plot_49 + + + + gnuplot_plot_50 + + + + + + + + + + + + + + + + diff --git a/doc/benchmark.md b/doc/benchmark.md index 244f25b9..0510b7ce 100644 --- a/doc/benchmark.md +++ b/doc/benchmark.md @@ -5,20 +5,10 @@ All benchmarks are done using [vtebench](https://github.com/alacritty/vtebench): ```sh -vtebench -h $(tput lines) -w $(tput cols) -b 104857600 alt-screen-random-write > ~/alt-random -vtebench -c -h $(tput lines) -w $(tput cols) -b 104857600 alt-screen-random-write > ~/alt-random-colors -vtebench -h $(tput lines) -w $(tput cols) -b 10485760 scrolling > ~/scrolling -vtebench -h $(tput lines) -w $(tput cols) -b 104857600 scrolling --fill-lines > ~/scrolling-filled-lines -vtebench -h $(tput lines) -w $(tput cols) -b 10485760 unicode-random-write > ~/unicode-random +./target/release/vtebench -b ./benchmarks ``` -They were "executed" using [benchmark.py](../scripts/benchmark.py), -which will load each file into memory, and then print it to the -terminal. This is done **20** times for each test. Then it calculates -the _mean_ and _standard deviation_ for each test. - - -## 2021-03-20 +## 2021-06-25 ### System @@ -40,14 +30,21 @@ Scrollback: 10000 lines ### Results +| Benchmark (times in ms) | Foot (GCC+PGO) 1.8.0 | Foot 1.8.0 | Alacritty 0.8.0 | URxvt 9.26 | XTerm 368 | +|-------------------------------|---------------------:|-----------:|----------------:|-----------:|----------:| +| cursor motion | 12.93 | 15.37 | 26.47 | 23.41 | 1304.00 | +| dense cells | 39.16 | 47.19 | 87.26 | 9110.00 | 10883.00 | +| light cells | 5.34 | 6.42 | 12.76 | 16.00 | 60.00 | +| scrollling | 144.26 | 139.93 | 133.98 | 117.52 | 3772.67 | +| scrolling bottom region | 130.81 | 125.34 | 116.10 | 117.31 | 3574.67 | +| scrolling bottom small region | 142.46 | 127.52 | 127.32 | 135.18 | 3572.67 | +| scrolling fullscreen | 5.43 | 5.27 | 12.06 | 11.97 | 118.62 | +| scrolling top region | 129.05 | 120.24 | 121.65 | 341.70 | 3567.33 | +| scrolling top small region | 121.59 | 109.82 | 137.03 | 219.96 | 3558.67 | +| unicode | 12.03 | 11.95 | 13.94 | 667.67 | 4905.67 | -| Benchmark | Foot (GCC+PGO) 1.7.0.r2 | Foot 1.7.0.r2 | Alacritty 0.7.2 | URxvt 9.22 | XTerm 366 | -|------------------------|------------------------:|--------------:|-------------------:|---------------:|---------------:| -| alt-random | 0.382s ±0.003 | 0.550s ±0.007 | 0.995s ±0.010 | 1.201s ±0.006 | 12.756s ±0.045 | -| alt-random-colors | 0.380s ±0.002 | 0.543s ±0.003 | 1.017s ±0.013 | 1.399s ±0.018 | 11.591s ±0.141 | -| scrolling | 1.302s ±0.019 | 1.284s ±0.052 | 1.107s ±0.028 | 1.097s ±0.015 | 37.537s ±0.121 | -| scrolling-filled-lines | 0.646s ±0.016 | 0.610s ±0.003 | 1.290s ±0.012 | 1.325s ±0.037 | 6.817s ±0.084 | -| unicode-random | 0.167s ±0.001 | 0.276s ±0.445 | 0.097s ±0.002 [^1] | 18.032s ±0.334 | 29.731s ±3.746 | + +![Graph of benchmark results for a beefy desktop system](benchmark-results-desktop.svg) ## 2021-03-20 From e365ac0b10686450329462aa3dac98b54e8d9b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 25 Jun 2021 10:24:55 +0200 Subject: [PATCH 03/17] doc: benchmark: add --dat to vtebench command line --- doc/benchmark.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/benchmark.md b/doc/benchmark.md index 0510b7ce..73573cc9 100644 --- a/doc/benchmark.md +++ b/doc/benchmark.md @@ -5,7 +5,7 @@ All benchmarks are done using [vtebench](https://github.com/alacritty/vtebench): ```sh -./target/release/vtebench -b ./benchmarks +./target/release/vtebench -b ./benchmarks --dat /tmp/ ``` ## 2021-06-25 From d206697001a8567f567c4313b313ce4968838283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 25 Jun 2021 10:56:40 +0200 Subject: [PATCH 04/17] doc: benchark: laptop results for 1.8.0 --- doc/benchmark-results-laptop.svg | 572 +++++++++++++++++++++++++++++++ doc/benchmark.md | 41 +-- 2 files changed, 585 insertions(+), 28 deletions(-) create mode 100644 doc/benchmark-results-laptop.svg diff --git a/doc/benchmark-results-laptop.svg b/doc/benchmark-results-laptop.svg new file mode 100644 index 00000000..df8a3ad7 --- /dev/null +++ b/doc/benchmark-results-laptop.svg @@ -0,0 +1,572 @@ + + + +Gnuplot +Produced by GNUPLOT 5.4 patchlevel 2 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0 + + + + + 200 + + + + + 400 + + + + + 600 + + + + + 800 + + + + + 1000 + + + + + 1200 + + + + + 1400 + + + + + 1600 + + + + + 1800 + + + + + 2000 + + + + + cursor_motion + + + + + dense_cells + + + + + light_cells + + + + + scrolling + + + + + scrolling_bottom_region + + + + + scrolling_bottom_small_region + + + + + scrolling_fullscreen + + + + + scrolling_top_region + + + + + scrolling_top_small_region + + + + + unicode + + + + + + + + + milliseconds (lower is better) + + + + + gnuplot_plot_1 + + + foot-pgo + + + + + + + + + + gnuplot_plot_2 + + + + + + + gnuplot_plot_3 + + + + + + + gnuplot_plot_4 + + + + + + + gnuplot_plot_5 + + + + + + + gnuplot_plot_6 + + + + + + + gnuplot_plot_7 + + + + + + + gnuplot_plot_8 + + + + + + + gnuplot_plot_9 + + + + + + + gnuplot_plot_10 + + + + + + + gnuplot_plot_11 + + + foot-no-pgo + + + + + + + + + + gnuplot_plot_12 + + + + + + + gnuplot_plot_13 + + + + + + + gnuplot_plot_14 + + + + + + + gnuplot_plot_15 + + + + + + + gnuplot_plot_16 + + + + + + + gnuplot_plot_17 + + + + + + + gnuplot_plot_18 + + + + + + + gnuplot_plot_19 + + + + + + + gnuplot_plot_20 + + + + + + + gnuplot_plot_21 + + + alacritty + + + + + + + + + + gnuplot_plot_22 + + + + + + + gnuplot_plot_23 + + + + + + + gnuplot_plot_24 + + + + + + + gnuplot_plot_25 + + + + + + + gnuplot_plot_26 + + + + + + + gnuplot_plot_27 + + + + + + + gnuplot_plot_28 + + + + + + + gnuplot_plot_29 + + + + + + + gnuplot_plot_30 + + + + + + + gnuplot_plot_31 + + + urxvt + + + + + + + + + + gnuplot_plot_32 + + + + + + + gnuplot_plot_33 + + + + + + + gnuplot_plot_34 + + + + + + + gnuplot_plot_35 + + + + + + + gnuplot_plot_36 + + + + + + + gnuplot_plot_37 + + + + + + + gnuplot_plot_38 + + + + + + + gnuplot_plot_39 + + + + + + + gnuplot_plot_40 + + + + gnuplot_plot_41 + + + xterm + + + + + + + + + + gnuplot_plot_42 + + + + gnuplot_plot_43 + + + + + + + gnuplot_plot_44 + + + + gnuplot_plot_45 + + + + gnuplot_plot_46 + + + + gnuplot_plot_47 + + + + + + + gnuplot_plot_48 + + + + gnuplot_plot_49 + + + + gnuplot_plot_50 + + + + + + + + + + + + + + + + diff --git a/doc/benchmark.md b/doc/benchmark.md index 73573cc9..8c1b5f9b 100644 --- a/doc/benchmark.md +++ b/doc/benchmark.md @@ -70,32 +70,17 @@ Scrollback=10000 lines ### Results -| Benchmark | Foot (GCC+PGO) 1.7.0.r2 | Foot (no PGO) 1.7.0.r2 | Alacritty 0.7.2 | URxvt 9.22 | St 0.8.4 | XTerm 366 | -|------------------------|------------------------:|-----------------------:|-------------------:|-----------------:|--------------:|----------------:| -| alt-random | 0.714s ±0.047 | 0.900s ±0.041 | 1.586s ±0.045 | 1.684s ±0.034 | 2.054s ±0.121 | 37.205s ±0.252 | -| alt-random-colors | 0.736s ±0.054 | 0.950s ±0.082 | 1.565s ±0.043 | 2.150s ±0.137 | 2.195s ±0.154 | 33.112s ±0.167 | -| scrolling | 1.593s ±0.070 | 1.559s ±0.055 | 1.517s ±0.079 | 1.462s ±0.052 | 3.308s ±0.133 | 134.432s ±0.436 | -| scrolling-filled-lines | 1.178s ±0.044 | 1.309s ±0.045 | 2.281s ±0.086 | 2.044s ±0.060 | 2.732s ±0.056 | 20.753s ±0.067 | -| unicode-random | 0.349s ±0.009 | 0.352s ±0.007 | 0.148s ±0.010 [^1] | 19.090s ±0.363 | crashed | 15.579s ±0.093 | +| Benchmark (times in ms) | Foot (GCC+PGO) 1.8.0 | Foot 1.8.0 | Alacritty 0.8.0 | URxvt 9.26 | XTerm 368 | +|-------------------------------|---------------------:|-----------:|----------------:|-----------:|----------:| +| cursor motion | 14.49 | 16.60 | 26.89 | 23.45 | 1303.38 | +| dense cells | 41.00 | 52.45 | 92.02 | 1486.57 | 11957.00 | +| light cells | 7.97 | 8.54 | 21.43 | 20.45 | 111.96 | +| scrollling | 158.85 | 158.90 | 148.06 | 138.98 | 10083.00 | +| scrolling bottom region | 153.83 | 151.38 | 142.13 | 151.30 | 9988.50 | +| scrolling bottom small region | 143.51 | 141.46 | 162.03 | 192.37 | 9938.00 | +| scrolling fullscreen | 11.56 | 11.75 | 22.96 | 21.49 | 295.40 | +| scrolling top region | 148.96 | 148.18 | 155.05 | 482.05 | 10036.00 | +| scrolling top small region | 144.26 | 149.76 | 159.40 | 321.69 | 9942.50 | +| unicode | 21.02 | 22.09 | 25.79 | 14959.00 | 88697.00 | -[^1]: [Alacritty and "unicode-random"](#alacritty-and-unicode-random) - - -# Alacritty and "unicode-random" - -Alacritty is actually **really** slow at rendering this (whether it is -fallback fonts in general, emojis, or something else, I don't know). - -I believe the reason it finishes the benchmark so quickly is because -it reads from the PTY in a separate thread, into a larger receive -buffer which is then consumed by the main thread. This allows the -client program to write its output much faster since it is no longer -stalling on a blocked PTY. - -This means Alacritty only needs to render a couple of frames since it -can reach the final VT state almost immediately. - -On the other hand, `cat`:ing the `unicode-random` test file in an -endless loop, or just manually scrolling up after the benchmark is -done is **slow**, which besides being felt (input lag), can be seen by -setting `debug.render_timer = true` in `alacritty.yml`. +![Graph of benchmark results for a laptop](benchmark-results-laptop.svg) From 0ff8f72a9d653269dd6127cc66322690c8a4b90f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 25 Jun 2021 20:42:23 +0200 Subject: [PATCH 05/17] =?UTF-8?q?vt:=20don=E2=80=99t=20reset=20utf8proc=20?= =?UTF-8?q?grapheme=20state=20when=20we=E2=80=99re=20not=20at=20a=20graphe?= =?UTF-8?q?me=20break?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 4 ++++ vt.c | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01cf1fdc..3934dadf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,10 @@ ### Deprecated ### Removed ### Fixed + +* Grapheme cluster state being reset between codepoints. + + ### Security ### Contributors diff --git a/vt.c b/vt.c index ec011cad..900ffd9d 100644 --- a/vt.c +++ b/vt.c @@ -639,10 +639,10 @@ action_utf8_print(struct terminal *term, wchar_t wc) #if defined(FOOT_GRAPHEME_CLUSTERING) if (grapheme_clustering) { /* Check if we're on a grapheme cluster break */ - /* Note: utf8proc fails to ZWJ */ - if (utf8proc_grapheme_break_stateful(last, wc, &term->vt.grapheme_state) && - last != 0x200d /* ZWJ */) + if (utf8proc_grapheme_break_stateful( + last, wc, &term->vt.grapheme_state)) { + term_reset_grapheme_state(term); goto out; } } @@ -682,6 +682,7 @@ action_utf8_print(struct terminal *term, wchar_t wc) { wc = precomposed; width = precomposed_width; + term_reset_grapheme_state(term); goto out; } } @@ -746,6 +747,7 @@ action_utf8_print(struct terminal *term, wchar_t wc) * character chains. Fall through here and print the * current zero-width character to the current cell */ LOG_WARN("maximum number of composed characters reached"); + term_reset_grapheme_state(term); goto out; } @@ -780,10 +782,11 @@ action_utf8_print(struct terminal *term, wchar_t wc) xassert(wc <= CELL_COMB_CHARS_HI); goto out; } - } + } else + term_reset_grapheme_state(term); + out: - term_reset_grapheme_state(term); if (width > 0) term_print(term, wc, width); } From 5dca0458a09ebc586fda49c50533976f055d8de2 Mon Sep 17 00:00:00 2001 From: Craig Barnes Date: Sat, 26 Jun 2021 22:15:09 +0100 Subject: [PATCH 06/17] log: add LOG_CLASS_NONE and use as initializer for log_level This means that logging will be completely disabled until log_init() has been called, which is useful to prevent log spam when running UNITTEST{} blocks in debug builds. Note that this doesn't change the default log level at runtime, which was already being set to LOG_CLASS_INFO in main.c and client.c. The new log level is also exposed to the command-line interface as `--log-level=none`, which allows disabling logging entirely. --- CHANGELOG.md | 6 +++ client.c | 32 ++++++------ completions/bash/foot | 2 +- completions/bash/footclient | 2 +- completions/fish/foot.fish | 2 +- completions/fish/footclient.fish | 2 +- completions/zsh/_foot | 2 +- completions/zsh/_footclient | 2 +- config.c | 6 ++- doc/foot.1.scd | 2 +- doc/footclient.1.scd | 2 +- log.c | 88 ++++++++++++++++---------------- log.h | 9 +++- main.c | 49 +++++++++--------- 14 files changed, 113 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3934dadf..0e6b7a73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,11 @@ ## Unreleased + ### Added + +* `--log-level=none` command-line option. + ### Changed ### Deprecated ### Removed @@ -40,6 +44,8 @@ ### Security ### Contributors +* [craigbarnes](https://codeberg.org/craigbarnes) + ## 1.8.0 diff --git a/client.c b/client.c index 9764da11..7b5cd770 100644 --- a/client.c +++ b/client.c @@ -58,22 +58,22 @@ print_usage(const char *prog_name) printf("Usage: %s [OPTIONS...] command [ARGS...]\n", prog_name); printf("\n"); printf("Options:\n"); - printf(" -t,--term=TERM value to set the environment variable TERM to (foot)\n" - " -T,--title=TITLE initial window title (foot)\n" - " -a,--app-id=ID window application ID (foot)\n" - " -w,--window-size-pixels=WIDTHxHEIGHT initial width and height, in pixels\n" - " -W,--window-size-chars=WIDTHxHEIGHT initial width and height, in characters\n" - " -m,--maximized start in maximized mode\n" - " -F,--fullscreen start in fullscreen mode\n" - " -L,--login-shell start shell as a login shell\n" - " -D,--working-directory=DIR directory to start in (CWD)\n" - " -s,--server-socket=PATH path to the server UNIX domain socket (default=$XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock)\n" - " -H,--hold remain open after child process exits\n" - " -N,--no-wait detach the client process from the running terminal, exiting immediately\n" - " -o,--override=[section.]key=value override configuration option\n" - " -d,--log-level={info|warning|error} log level (info)\n" - " -l,--log-colorize=[{never|always|auto}] enable/disable colorization of log output on stderr\n" - " -v,--version show the version number and quit\n"); + printf(" -t,--term=TERM value to set the environment variable TERM to (foot)\n" + " -T,--title=TITLE initial window title (foot)\n" + " -a,--app-id=ID window application ID (foot)\n" + " -w,--window-size-pixels=WIDTHxHEIGHT initial width and height, in pixels\n" + " -W,--window-size-chars=WIDTHxHEIGHT initial width and height, in characters\n" + " -m,--maximized start in maximized mode\n" + " -F,--fullscreen start in fullscreen mode\n" + " -L,--login-shell start shell as a login shell\n" + " -D,--working-directory=DIR directory to start in (CWD)\n" + " -s,--server-socket=PATH path to the server UNIX domain socket (default=$XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock)\n" + " -H,--hold remain open after child process exits\n" + " -N,--no-wait detach the client process from the running terminal, exiting immediately\n" + " -o,--override=[section.]key=value override configuration option\n" + " -d,--log-level={info|warning|error|none} log level (info)\n" + " -l,--log-colorize=[{never|always|auto}] enable/disable colorization of log output on stderr\n" + " -v,--version show the version number and quit\n"); } static bool NOINLINE diff --git a/completions/bash/foot b/completions/bash/foot index f03a9f53..f9d9d0fd 100644 --- a/completions/bash/foot +++ b/completions/bash/foot @@ -68,7 +68,7 @@ _foot() which fc-list > /dev/null || return 1 COMPREPLY=( $(compgen -W "$(fc-list : family | sed 's/,/\n/g' | uniq | tr -d ' ')" -- ${cur}) ) elif [[ ${prev} == '--log-level' ]] ; then - COMPREPLY=( $(compgen -W "error warning info" -- ${cur}) ) + COMPREPLY=( $(compgen -W "none error warning info" -- ${cur}) ) elif [[ ${prev} == '--log-colorize' ]] ; then COMPREPLY=( $(compgen -W "never always auto" -- ${cur}) ) elif [[ ${prev} =~ ^(--app-id|--help|--override|--title|--version|--window-size-chars|--window-size-pixels|--check-config)$ ]] ; then diff --git a/completions/bash/footclient b/completions/bash/footclient index 8e0aa017..fea45945 100644 --- a/completions/bash/footclient +++ b/completions/bash/footclient @@ -59,7 +59,7 @@ _footclient() which toe > /dev/null || return 1 COMPREPLY=( $(compgen -W "$(toe -a | awk '$1 ~ /[+]/ {next}; {print $1}')" -- ${cur}) ) elif [[ ${prev} == '--log-level' ]] ; then - COMPREPLY=( $(compgen -W "error warning info" -- ${cur}) ) + COMPREPLY=( $(compgen -W "none error warning info" -- ${cur}) ) elif [[ ${prev} == '--log-colorize' ]] ; then COMPREPLY=( $(compgen -W "never always auto" -- ${cur}) ) elif [[ ${prev} =~ ^(--app-id|--help|--override|--title|--version|--window-size-chars|--window-size-pixels|)$ ]] ; then diff --git a/completions/fish/foot.fish b/completions/fish/foot.fish index dbec61e7..81c7da61 100644 --- a/completions/fish/foot.fish +++ b/completions/fish/foot.fish @@ -15,7 +15,7 @@ complete -c foot -x -s W -l window-size-chars complete -c foot -F -s s -l server -d "run as server; open terminals by running footclient" complete -c foot -s H -l hold -d "remain open after child process exits" complete -c foot -r -s p -l print-pid -d "print PID to this file or FD when up and running (server mode only)" -complete -c foot -x -s d -l log-level -a "info warning error" -d "log-level (info)" +complete -c foot -x -s d -l log-level -a "info warning error none" -d "log-level (info)" complete -c foot -x -s l -l log-colorize -a "always never auto" -d "enable or disable colorization of log output on stderr" complete -c foot -s S -l log-no-syslog -d "disable syslog logging (server mode only)" complete -c foot -s v -l version -d "show the version number and quit" diff --git a/completions/fish/footclient.fish b/completions/fish/footclient.fish index 9b8caa84..8133f663 100644 --- a/completions/fish/footclient.fish +++ b/completions/fish/footclient.fish @@ -12,7 +12,7 @@ complete -c footclient -F -s s -l server-socket complete -c footclient -s H -l hold -d "remain open after child process exits" complete -c footclient -s N -l no-wait -d "detach the client process from the running terminal, exiting immediately" complete -c footclient -x -s o -l override -d "configuration option to override, in form SECTION.KEY=VALUE" -complete -c footclient -x -s d -l log-level -a "info warning error" -d "log-level (info)" +complete -c footclient -x -s d -l log-level -a "info warning error none" -d "log-level (info)" complete -c footclient -x -s l -l log-colorize -a "always never auto" -d "enable or disable colorization of log output on stderr" complete -c footclient -s v -l version -d "show the version number and quit" complete -c footclient -s h -l help -d "show help message and quit" diff --git a/completions/zsh/_foot b/completions/zsh/_foot index b3f9b42c..0f184cc0 100644 --- a/completions/zsh/_foot +++ b/completions/zsh/_foot @@ -18,7 +18,7 @@ _arguments \ '(-s --server)'{-s,--server}'[run as server; open terminals by running footclient]:server:_files' \ '(-H --hold)'{-H,--hold}'[remain open after child process exits]' \ '(-p --print-pid)'{-p,--print-pid}'[print PID to this file or FD when up and running (server mode only)]:pidfile:_files' \ - '(-d --log-level)'{-d,--log-level}'[log level (info)]:loglevel:(info warning error)' \ + '(-d --log-level)'{-d,--log-level}'[log level (info)]:loglevel:(info warning error none)' \ '(-l --log-colorize)'{-l,--log-colorize}'[enable or disable colorization of log output on stderr]:logcolor:(never always auto)' \ '(-S --log-no-syslog)'{-s,--log-no-syslog}'[disable syslog logging (server mode only)]' \ '(-v --version)'{-v,--version}'[show the version number and quit]' \ diff --git a/completions/zsh/_footclient b/completions/zsh/_footclient index 3889c488..81b2ea95 100644 --- a/completions/zsh/_footclient +++ b/completions/zsh/_footclient @@ -15,7 +15,7 @@ _arguments \ '(-H --hold)'{-H,--hold}'[remain open after child process exits]' \ '(-N --no-wait)'{-N,--no-wait}'[detach the client process from the running terminal, exiting immediately]' \ '(-o --override)'{-o,--override}'[configuration option to override, in form SECTION.KEY=VALUE]:()' \ - '(-d --log-level)'{-d,--log-level}'[log level (info)]:loglevel:(info warning error)' \ + '(-d --log-level)'{-d,--log-level}'[log level (info)]:loglevel:(info warning error none)' \ '(-l --log-colorize)'{-l,--log-colorize}'[enable or disable colorization of log output on stderr]:logcolor:(never always auto)' \ '(-v --version)'{-v,--version}'[show the version number and quit]' \ '(-h --help)'{-h,--help}'[show help message and quit]' \ diff --git a/config.c b/config.c index a95f8485..c417fb19 100644 --- a/config.c +++ b/config.c @@ -103,8 +103,10 @@ log_and_notify(struct config *conf, enum log_class log_class, case LOG_CLASS_INFO: case LOG_CLASS_DEBUG: - BUG("unsupported log class: %d", log_class); - break; + case LOG_CLASS_NONE: + default: + BUG("unsupported log class: %d", (int)log_class); + return; } va_list va1, va2; diff --git a/doc/foot.1.scd b/doc/foot.1.scd index 80203ad9..ab34a2dd 100644 --- a/doc/foot.1.scd +++ b/doc/foot.1.scd @@ -126,7 +126,7 @@ the foot command line This option can only be used in combination with *-s*,*--server*. -*-d*,*--log-level*={*info*,*warning*,*error*} +*-d*,*--log-level*={*info*,*warning*,*error*,*none*} Log level, used both for log output on stderr as well as syslog. Default: _info_. diff --git a/doc/footclient.1.scd b/doc/footclient.1.scd index 8cb05316..a002a899 100644 --- a/doc/footclient.1.scd +++ b/doc/footclient.1.scd @@ -69,7 +69,7 @@ terminal has terminated. Override an option set in the configuration file. If _SECTION_ is not given, defaults to _main_. -*-d*,*--log-level*={*info*,*warning*,*error*} +*-d*,*--log-level*={*info*,*warning*,*error*,*none*} Log level, used both for log output on stderr as well as syslog. Default: _info_. diff --git a/log.c b/log.c index fba53876..9dfc842d 100644 --- a/log.c +++ b/log.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -15,15 +16,19 @@ static bool colorize = false; static bool do_syslog = true; -static enum log_class log_level = LOG_CLASS_INFO; +static enum log_class log_level = LOG_CLASS_NONE; -static const char log_level_map[][8] = { - [LOG_CLASS_ERROR] = "error", - [LOG_CLASS_WARNING] = "warning", - [LOG_CLASS_INFO] = "info", -#if defined(_DEBUG) - [LOG_CLASS_DEBUG] = "debug", -#endif +static const struct { + const char name[8]; + const char log_prefix[7]; + uint8_t color; + int syslog_equivalent; +} log_level_map[] = { + [LOG_CLASS_NONE] = {"none", "none", 5, -1}, + [LOG_CLASS_ERROR] = {"error", " err", 31, LOG_ERR}, + [LOG_CLASS_WARNING] = {"warning", "warn", 33, LOG_WARNING}, + [LOG_CLASS_INFO] = {"info", "info", 97, LOG_INFO}, + [LOG_CLASS_DEBUG] = {"debug", " dbg", 36, LOG_DEBUG}, }; void @@ -35,20 +40,14 @@ log_init(enum log_colorize _colorize, bool _do_syslog, [LOG_FACILITY_DAEMON] = LOG_DAEMON, }; - static const int level_map[] = { - [LOG_CLASS_ERROR] = LOG_ERR, - [LOG_CLASS_WARNING] = LOG_WARNING, - [LOG_CLASS_INFO] = LOG_INFO, - [LOG_CLASS_DEBUG] = LOG_DEBUG, - }; - colorize = _colorize == LOG_COLORIZE_NEVER ? false : _colorize == LOG_COLORIZE_ALWAYS ? true : isatty(STDERR_FILENO); do_syslog = _do_syslog; log_level = _log_level; - if (do_syslog) { + int slvl = log_level_map[_log_level].syslog_equivalent; + if (do_syslog && slvl != -1) { openlog(NULL, /*LOG_PID*/0, facility_map[syslog_facility]); - setlogmask(LOG_UPTO(level_map[_log_level])); + setlogmask(LOG_UPTO(slvl)); } } @@ -63,34 +62,31 @@ static void _log(enum log_class log_class, const char *module, const char *file, int lineno, const char *fmt, int sys_errno, va_list va) { + xassert(log_class > LOG_CLASS_NONE); + xassert(log_class < ALEN(log_level_map)); + if (log_class > log_level) return; - const char *class = "abcd"; - int class_clr = 0; - switch (log_class) { - case LOG_CLASS_ERROR: class = " err"; class_clr = 31; break; - case LOG_CLASS_WARNING: class = "warn"; class_clr = 33; break; - case LOG_CLASS_INFO: class = "info"; class_clr = 97; break; - case LOG_CLASS_DEBUG: class = " dbg"; class_clr = 36; break; - } + const char *prefix = log_level_map[log_class].log_prefix; + unsigned int class_clr = log_level_map[log_class].color; char clr[16]; - snprintf(clr, sizeof(clr), "\033[%dm", class_clr); - fprintf(stderr, "%s%s%s: ", colorize ? clr : "", class, colorize ? "\033[0m" : ""); + xsnprintf(clr, sizeof(clr), "\033[%um", class_clr); + fprintf(stderr, "%s%s%s: ", colorize ? clr : "", prefix, colorize ? "\033[0m" : ""); if (colorize) - fprintf(stderr, "\033[2m"); + fputs("\033[2m", stderr); fprintf(stderr, "%s:%d: ", file, lineno); if (colorize) - fprintf(stderr, "\033[0m"); + fputs("\033[0m", stderr); vfprintf(stderr, fmt, va); if (sys_errno != 0) fprintf(stderr, ": %s", strerror(sys_errno)); - fprintf(stderr, "\n"); + fputc('\n', stderr); } static void @@ -98,19 +94,14 @@ _sys_log(enum log_class log_class, const char *module, const char UNUSED *file, int UNUSED lineno, const char *fmt, int sys_errno, va_list va) { + xassert(log_class > LOG_CLASS_NONE); + xassert(log_class < ALEN(log_level_map)); + if (!do_syslog) return; /* Map our log level to syslog's level */ - int level = -1; - switch (log_class) { - case LOG_CLASS_ERROR: level = LOG_ERR; break; - case LOG_CLASS_WARNING: level = LOG_WARNING; break; - case LOG_CLASS_INFO: level = LOG_INFO; break; - case LOG_CLASS_DEBUG: level = LOG_DEBUG; break; - } - - xassert(level != -1); + int level = log_level_map[log_class].syslog_equivalent; char msg[4096]; int n = vsnprintf(msg, sizeof(msg), fmt, va); @@ -185,14 +176,25 @@ log_errno_provided(enum log_class log_class, const char *module, va_end(va); } +static size_t +map_len(void) +{ + size_t len = ALEN(log_level_map); +#ifndef _DEBUG + /* Exclude "debug" entry for non-debug builds */ + len--; +#endif + return len; +} + int log_level_from_string(const char *str) { if (unlikely(str[0] == '\0')) return -1; - for (int i = 0, n = ALEN(log_level_map); i < n; i++) - if (strcmp(str, log_level_map[i]) == 0) + for (int i = 0, n = map_len(); i < n; i++) + if (strcmp(str, log_level_map[i].name) == 0) return i; return -1; @@ -205,8 +207,8 @@ log_level_string_hint(void) if (buf[0] != '\0') return buf; - for (size_t i = 0, pos = 0, n = ALEN(log_level_map); i < n; i++) { - const char *entry = log_level_map[i]; + for (size_t i = 0, pos = 0, n = map_len(); i < n; i++) { + const char *entry = log_level_map[i].name; const char *delim = (i + 1 < n) ? ", " : ""; pos += xsnprintf(buf + pos, sizeof(buf) - pos, "'%s'%s", entry, delim); } diff --git a/log.h b/log.h index 367fd60e..dc458199 100644 --- a/log.h +++ b/log.h @@ -5,7 +5,14 @@ enum log_colorize { LOG_COLORIZE_NEVER, LOG_COLORIZE_ALWAYS, LOG_COLORIZE_AUTO }; enum log_facility { LOG_FACILITY_USER, LOG_FACILITY_DAEMON }; -enum log_class { LOG_CLASS_ERROR, LOG_CLASS_WARNING, LOG_CLASS_INFO, LOG_CLASS_DEBUG }; + +enum log_class { + LOG_CLASS_NONE, + LOG_CLASS_ERROR, + LOG_CLASS_WARNING, + LOG_CLASS_INFO, + LOG_CLASS_DEBUG +}; void log_init(enum log_colorize colorize, bool do_syslog, enum log_facility syslog_facility, enum log_class log_level); diff --git a/main.c b/main.c index bbe89052..f58bdb52 100644 --- a/main.c +++ b/main.c @@ -61,27 +61,27 @@ print_usage(const char *prog_name) "Usage: %s [OPTIONS...] command [ARGS...]\n" "\n" "Options:\n" - " -c,--config=PATH load configuration from PATH ($XDG_CONFIG_HOME/foot/foot.ini)\n" - " -C,--check-config verify configuration, exit with 0 if ok, otherwise exit with 1\n" - " -o,--override=[section.]key=value override configuration option\n" - " -f,--font=FONT comma separated list of fonts in fontconfig format (monospace)\n" - " -t,--term=TERM value to set the environment variable TERM to (%s)\n" - " -T,--title=TITLE initial window title (foot)\n" - " -a,--app-id=ID window application ID (foot)\n" - " -m,--maximized start in maximized mode\n" - " -F,--fullscreen start in fullscreen mode\n" - " -L,--login-shell start shell as a login shell\n" - " -D,--working-directory=DIR directory to start in (CWD)\n" - " -w,--window-size-pixels=WIDTHxHEIGHT initial width and height, in pixels\n" - " -W,--window-size-chars=WIDTHxHEIGHT initial width and height, in characters\n" - " -s,--server[=PATH] run as a server (use 'footclient' to start terminals).\n" - " Without PATH, $XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock will be used.\n" - " -H,--hold remain open after child process exits\n" - " -p,--print-pid=FILE|FD print PID to file or FD (only applicable in server mode)\n" - " -d,--log-level={info|warning|error} log level (info)\n" - " -l,--log-colorize=[{never|always|auto}] enable/disable colorization of log output on stderr\n" - " -s,--log-no-syslog disable syslog logging (only applicable in server mode)\n" - " -v,--version show the version number and quit\n", + " -c,--config=PATH load configuration from PATH ($XDG_CONFIG_HOME/foot/foot.ini)\n" + " -C,--check-config verify configuration, exit with 0 if ok, otherwise exit with 1\n" + " -o,--override=[section.]key=value override configuration option\n" + " -f,--font=FONT comma separated list of fonts in fontconfig format (monospace)\n" + " -t,--term=TERM value to set the environment variable TERM to (%s)\n" + " -T,--title=TITLE initial window title (foot)\n" + " -a,--app-id=ID window application ID (foot)\n" + " -m,--maximized start in maximized mode\n" + " -F,--fullscreen start in fullscreen mode\n" + " -L,--login-shell start shell as a login shell\n" + " -D,--working-directory=DIR directory to start in (CWD)\n" + " -w,--window-size-pixels=WIDTHxHEIGHT initial width and height, in pixels\n" + " -W,--window-size-chars=WIDTHxHEIGHT initial width and height, in characters\n" + " -s,--server[=PATH] run as a server (use 'footclient' to start terminals).\n" + " Without PATH, $XDG_RUNTIME_DIR/foot-$WAYLAND_DISPLAY.sock will be used.\n" + " -H,--hold remain open after child process exits\n" + " -p,--print-pid=FILE|FD print PID to file or FD (only applicable in server mode)\n" + " -d,--log-level={info|warning|error|none} log level (info)\n" + " -l,--log-colorize=[{never|always|auto}] enable/disable colorization of log output on stderr\n" + " -s,--log-no-syslog disable syslog logging (only applicable in server mode)\n" + " -v,--version show the version number and quit\n", prog_name, prog_name, DEFAULT_TERM); } @@ -383,11 +383,14 @@ main(int argc, char *const *argv) log_init(log_colorize, as_server && log_syslog, as_server ? LOG_FACILITY_DAEMON : LOG_FACILITY_USER, log_level); - _Static_assert(LOG_CLASS_ERROR + 1 == FCFT_LOG_CLASS_ERROR, + _Static_assert((int)LOG_CLASS_ERROR == (int)FCFT_LOG_CLASS_ERROR, "fcft log level enum offset"); _Static_assert((int)LOG_COLORIZE_ALWAYS == (int)FCFT_LOG_COLORIZE_ALWAYS, "fcft colorize enum mismatch"); - fcft_log_init((enum fcft_log_colorize)log_colorize, as_server && log_syslog, log_level + 1); + fcft_log_init( + (enum fcft_log_colorize)log_colorize, + as_server && log_syslog, + (enum fcft_log_class)log_level); argc -= optind; argv += optind; From 117e24dbf4e5a54c8209f7415a97dbd2da906cd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 28 Jun 2021 22:33:57 +0200 Subject: [PATCH 07/17] term: destroy: free URLs before free:ing the grids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes use-after-free when destroying a terminal with “live” URLs (i.e. when URL mode is active). --- terminal.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/terminal.c b/terminal.c index cb836fef..6c175147 100644 --- a/terminal.c +++ b/terminal.c @@ -1425,10 +1425,10 @@ term_destroy(struct terminal *term) } mtx_unlock(&term->render.workers.lock); + urls_reset(term); + free(term->vt.osc.data); free(term->vt.osc8.uri); - grid_free(&term->normal); - grid_free(&term->alt); composed_free(term->composed); @@ -1471,9 +1471,11 @@ term_destroy(struct terminal *term) sixel_fini(term); - urls_reset(term); term_ime_reset(term); + grid_free(&term->normal); + grid_free(&term->alt); + free(term->foot_exe); free(term->cwd); From a09f92817541c5935fda51f11004b7baae06f2ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 28 Jun 2021 22:34:52 +0200 Subject: [PATCH 08/17] =?UTF-8?q?input:=20ignore=20=E2=80=98unused?= =?UTF-8?q?=E2=80=99=20URL=20key=20bindings=20when=20mapping=20bindings=20?= =?UTF-8?q?to=20current=20keymap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a temporary fix for #614. Long term fix is to remove the ‘replaced’ bindings from the array at config time. --- input.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/input.c b/input.c index e33fdff4..7db3ca85 100644 --- a/input.c +++ b/input.c @@ -551,10 +551,8 @@ convert_url_bindings(const struct config *conf, struct seat *seat) { for (size_t i = 0; i < conf->bindings.url.count; i++) { const struct config_key_binding *binding = &conf->bindings.url.arr[i]; -#if 0 if (binding->action == BIND_ACTION_URL_NONE) continue; -#endif convert_key_binding(seat, binding, &seat->kbd.bindings.url); } } From 3e74482d6c709b09477fdab3a0be6ba678c4accb Mon Sep 17 00:00:00 2001 From: Craig Barnes Date: Tue, 29 Jun 2021 08:47:46 +0100 Subject: [PATCH 09/17] terminfo: add Tc, setrgbf and setrgbb capabilities These extensions are used by tmux and neovim, in order to make use of 24-bit colors without facing the problems that plague the `RGB` capability. This should allow 24-bit colors to work "out of the box" in tmux, without the usual workaround of adding: set-option -ga terminal-overrides ",foot*:Tc" ...to ~/.tmux.conf. See also: * https://github.com/kovidgoyal/kitty/commit/18fe2e8dfa34038aabd5c3a2fdb3624e2b27932a#commitcomment-31373962 * https://github.com/neovim/neovim/blob/f83c25942dd8b94ad5218ce78b9e6fb86d2f0358/runtime/doc/term.txt#L123-L139 * https://github.com/tmux/tmux/blob/b1a8c0fe022e99cffb0fb4f321740464f35bc6b9/CHANGES#L988-L989 Closes #615 --- CHANGELOG.md | 4 ++++ foot.info | 3 +++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6b7a73..a44d8105 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ ### Added * `--log-level=none` command-line option. +* `Tc`, `setrgbf` and `setrgbb` capabilities in `foot` and `foot-direct` + terminfo entries. This should make 24-bit RGB colors work in tmux and + neovim, without the need for config hacks or detection heuristics + (https://codeberg.org/dnkl/foot/issues/615). ### Changed ### Deprecated diff --git a/foot.info b/foot.info index 41c23593..37932204 100644 --- a/foot.info +++ b/foot.info @@ -24,6 +24,7 @@ foot+base|foot base fragment, xenl, AX, XT, + Tc, cols#80, it#8, lines#24, @@ -255,6 +256,8 @@ foot+base|foot base fragment, rs1=\Ec, rs2=\E[!p\E[?3;4l\E[4l\E>, sc=\E7, + setrgbb=\E[48\:2\:\:%p1%d\:%p2%d\:%p3%dm, + setrgbf=\E[38\:2\:\:%p1%d\:%p2%d\:%p3%dm, sgr0=\E(B\E[m, sgr=%?%p9%t\E(0%e\E(B%;\E[0%?%p6%t;1%;%?%p5%t;2%;%?%p2%t;4%;%?%p1%p3%|%t;7%;%?%p4%t;5%;%?%p7%t;8%;m, sitm=\E[3m, From cf46acc68f221aeac71bd2ef60a3beeeb0b3be47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Jun 2021 18:01:03 +0200 Subject: [PATCH 10/17] =?UTF-8?q?render:=20don=E2=80=99t=20look=20at=20gly?= =?UTF-8?q?phs[0]->cols=20when=20determining=20if=20overflow=20should=20be?= =?UTF-8?q?=20allowed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use ‘cell_cols’ instead, which is the number of cells we’ll actually be using. --- render.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render.c b/render.c index d3e03b63..499303f0 100644 --- a/render.c +++ b/render.c @@ -617,7 +617,7 @@ render_cell(struct terminal *term, pixman_image_t *pix, */ if (term->conf->tweak.allow_overflowing_double_width_glyphs && ((glyph_count > 0 && - glyphs[0]->cols == 1 && + cell_cols == 1 && glyphs[0]->width >= term->cell_width * 15 / 10 && glyphs[0]->width < 3 * term->cell_width && col < term->cols - 1) || From 149c52bd4496eb4dd13eaa53bec690c60499502a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 29 Jun 2021 18:08:15 +0200 Subject: [PATCH 11/17] =?UTF-8?q?config:=20remove=20replaced/removed=20key?= =?UTF-8?q?=20bindings,=20instead=20of=20marking=20as=20=E2=80=98unused?= =?UTF-8?q?=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of keeping removed/replaced key bindings in the key binding array (marked as ‘unused’), remove them, by compacting the array. The invariant is thus that there should be *no* entries in the key binding list with the `BIND_ACTION_NONE` for action. Add code to debug builds that verifies this, plus a unit test. Closes #614 --- CHANGELOG.md | 2 + config.c | 158 ++++++++++++++++++++++++++++++++++++++++++--------- input.c | 6 -- 3 files changed, 134 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a44d8105..78016bae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ ### Fixed * Grapheme cluster state being reset between codepoints. +* Regression: custom URL key bindings not working + (https://codeberg.org/dnkl/foot/issues/614). ### Security diff --git a/config.c b/config.c index c417fb19..2fa9100e 100644 --- a/config.c +++ b/config.c @@ -1662,6 +1662,43 @@ pipe_argv_from_string(const char *value, char ***argv, return remove_len; } +static void +remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, + int action, char **pipe_argv) +{ + size_t remove_first_idx = 0; + size_t remove_count = 0; + + for (size_t i = 0; i < bindings->count; i++) { + struct config_key_binding *binding = &bindings->arr[i]; + + if (binding->action == action && + ((binding->pipe.argv.args == NULL && pipe_argv == NULL) || + (binding->pipe.argv.args != NULL && pipe_argv != NULL && + argv_compare(binding->pipe.argv.args, pipe_argv) == 0))) + { + if (remove_count++ == 0) + remove_first_idx = i; + + xassert(remove_first_idx + remove_count - 1 == i); + + if (binding->pipe.master_copy) + free_argv(&binding->pipe.argv); + } + } + + if (remove_count == 0) + return; + + size_t move_count = bindings->count - (remove_first_idx + remove_count); + + memmove( + &bindings->arr[remove_first_idx], + &bindings->arr[remove_first_idx + remove_count], + move_count * sizeof(bindings->arr[0])); + bindings->count -= remove_count; +} + static bool NOINLINE parse_key_binding_section( const char *section, const char *key, const char *value, @@ -1688,17 +1725,7 @@ parse_key_binding_section( /* Unset binding */ if (strcasecmp(value, "none") == 0) { - for (size_t i = 0; i < bindings->count; i++) { - struct config_key_binding *binding = &bindings->arr[i]; - - if (binding->action != action) - continue; - - if (binding->pipe.master_copy) - free_argv(&binding->pipe.argv); - binding->action = BIND_ACTION_NONE; - } - + remove_action_from_key_bindings_list(bindings, action, pipe_argv); free(pipe_argv); return true; } @@ -1715,21 +1742,7 @@ parse_key_binding_section( return false; } - /* Remove existing bindings for this action+pipe */ - for (size_t i = 0; i < bindings->count; i++) { - struct config_key_binding *binding = &bindings->arr[i]; - - if (binding->action == action && - ((binding->pipe.argv.args == NULL && pipe_argv == NULL) || - (binding->pipe.argv.args != NULL && pipe_argv != NULL && - argv_compare(binding->pipe.argv.args, pipe_argv) == 0))) - { - - if (binding->pipe.master_copy) - free_argv(&binding->pipe.argv); - binding->action = BIND_ACTION_NONE; - } - } + remove_action_from_key_bindings_list(bindings, action, pipe_argv); /* Emit key bindings */ size_t ofs = bindings->count; @@ -1767,6 +1780,90 @@ parse_key_binding_section( return false; } +UNITTEST +{ + enum test_actions { + TEST_ACTION_NONE, + TEST_ACTION_FOO, + TEST_ACTION_BAR, + TEST_ACTION_COUNT, + }; + + const char *const map[] = { + [TEST_ACTION_NONE] = NULL, + [TEST_ACTION_FOO] = "foo", + [TEST_ACTION_BAR] = "bar", + }; + + struct config conf = {0}; + struct config_key_binding_list bindings = {0}; + + /* + * ADD foo=Escape + * + * This verifies we can bind a single key combo to an action. + */ + xassert(parse_key_binding_section( + "", "foo", "Escape", ALEN(map), map, &bindings, &conf, "", 0)); + xassert(bindings.count == 1); + xassert(bindings.arr[0].action == TEST_ACTION_FOO); + xassert(bindings.arr[0].sym == XKB_KEY_Escape); + + /* + * ADD bar=Control+g Control+Shift+x + * + * This verifies we can bind multiple key combos to an action. + */ + xassert(parse_key_binding_section( + "", "bar", "Control+g Control+Shift+x", ALEN(map), map, + &bindings, &conf, "", 0)); + xassert(bindings.count == 3); + xassert(bindings.arr[0].action == TEST_ACTION_FOO); + xassert(bindings.arr[1].action == TEST_ACTION_BAR); + xassert(bindings.arr[1].sym == XKB_KEY_g); + xassert(bindings.arr[1].modifiers.ctrl); + xassert(bindings.arr[2].action == TEST_ACTION_BAR); + xassert(bindings.arr[2].sym == XKB_KEY_x); + xassert(bindings.arr[2].modifiers.ctrl && bindings.arr[2].modifiers.shift); + + /* + * REPLACE foo with foo=Mod+v Shift+q + * + * This verifies we can update a single-combo action with multiple + * key combos. + */ + xassert(parse_key_binding_section( + "", "foo", "Mod1+v Shift+q", ALEN(map), map, + &bindings, &conf, "", 0)); + xassert(bindings.count == 4); + xassert(bindings.arr[0].action == TEST_ACTION_BAR); + xassert(bindings.arr[1].action == TEST_ACTION_BAR); + xassert(bindings.arr[2].action == TEST_ACTION_FOO); + xassert(bindings.arr[2].sym == XKB_KEY_v); + xassert(bindings.arr[2].modifiers.alt); + xassert(bindings.arr[3].action == TEST_ACTION_FOO); + xassert(bindings.arr[3].sym == XKB_KEY_q); + xassert(bindings.arr[3].modifiers.shift); + + /* + * REMOVE bar + */ + xassert(parse_key_binding_section( + "", "bar", "none", ALEN(map), map, &bindings, &conf, "", 0)); + xassert(bindings.count == 2); + xassert(bindings.arr[0].action == TEST_ACTION_FOO); + xassert(bindings.arr[1].action == TEST_ACTION_FOO); + + /* + * REMOVE foo + */ + xassert(parse_key_binding_section( + "", "foo", "none", ALEN(map), map, &bindings, &conf, "", 0)); + xassert(bindings.count == 0); + + free(bindings.arr); +} + static bool parse_section_key_bindings( const char *key, const char *value, struct config *conf, @@ -2852,6 +2949,15 @@ out: } } +#if defined(_DEBUG) + for (size_t i = 0; i < conf->bindings.key.count; i++) + xassert(conf->bindings.key.arr[i].action != BIND_ACTION_NONE); + for (size_t i = 0; i < conf->bindings.search.count; i++) + xassert(conf->bindings.search.arr[i].action != BIND_ACTION_SEARCH_NONE); + for (size_t i = 0; i < conf->bindings.url.count; i++) + xassert(conf->bindings.url.arr[i].action != BIND_ACTION_URL_NONE); +#endif + free(conf_file.path); if (conf_file.fd >= 0) close(conf_file.fd); diff --git a/input.c b/input.c index 7db3ca85..9cf18b53 100644 --- a/input.c +++ b/input.c @@ -529,8 +529,6 @@ convert_key_bindings(const struct config *conf, struct seat *seat) { for (size_t i = 0; i < conf->bindings.key.count; i++) { const struct config_key_binding *binding = &conf->bindings.key.arr[i]; - if (binding->action == BIND_ACTION_NONE) - continue; convert_key_binding(seat, binding, &seat->kbd.bindings.key); } } @@ -540,8 +538,6 @@ convert_search_bindings(const struct config *conf, struct seat *seat) { for (size_t i = 0; i < conf->bindings.search.count; i++) { const struct config_key_binding *binding = &conf->bindings.search.arr[i]; - if (binding->action == BIND_ACTION_SEARCH_NONE) - continue; convert_key_binding(seat, binding, &seat->kbd.bindings.search); } } @@ -551,8 +547,6 @@ convert_url_bindings(const struct config *conf, struct seat *seat) { for (size_t i = 0; i < conf->bindings.url.count; i++) { const struct config_key_binding *binding = &conf->bindings.url.arr[i]; - if (binding->action == BIND_ACTION_URL_NONE) - continue; convert_key_binding(seat, binding, &seat->kbd.bindings.url); } } From 88fb8429b0d0abeed7087373241dd57918550974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Jun 2021 19:10:37 +0200 Subject: [PATCH 12/17] config: NOINLINE a couple of functions doing tll operations --- config.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config.c b/config.c index 2fa9100e..5fd68a2d 100644 --- a/config.c +++ b/config.c @@ -193,7 +193,7 @@ struct path_component { }; typedef tll(struct path_component) path_components_t; -static void +static void NOINLINE path_component_add(path_components_t *components, const char *comp, int fd) { xassert(comp != NULL); @@ -203,14 +203,14 @@ path_component_add(path_components_t *components, const char *comp, int fd) tll_push_back(*components, pc); } -static void +static void NOINLINE path_component_destroy(struct path_component *component) { xassert(component->fd >= 0); close(component->fd); } -static void +static void NOINLINE path_components_destroy(path_components_t *components) { tll_foreach(*components, it) { @@ -1662,7 +1662,7 @@ pipe_argv_from_string(const char *value, char ***argv, return remove_len; } -static void +static void NOINLINE remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, int action, char **pipe_argv) { From 55bd3cfd623a86044fd028787c216810ee274af7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Jun 2021 21:06:21 +0200 Subject: [PATCH 13/17] config: add unit test for config_clone() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’re mainly interested in seeing that we don’t leak memory, or double free heap allocated data. Valgrind and/or gcc/clang sanitizers are best at this, hence the very few asserts in the test itself. --- config.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/config.c b/config.c index 5fd68a2d..372e9f9c 100644 --- a/config.c +++ b/config.c @@ -3140,6 +3140,27 @@ config_clone(const struct config *old) return conf; } +UNITTEST +{ + struct config original; + user_notifications_t nots = tll_init(); + config_override_t overrides = tll_init(); + + bool ret = config_load(&original, "/dev/null", ¬s, &overrides, false); + xassert(ret); + + struct config *clone = config_clone(&original); + xassert(clone != NULL); + xassert(clone != &original); + + config_free(original); + config_free(*clone); + free(clone); + + tll_free(overrides); + tll_free(nots); +} + void config_free(struct config conf) { From 031e8f59877f4ad25a5a4116342d66f9aac56c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 29 Jun 2021 18:45:27 +0200 Subject: [PATCH 14/17] vt: limit grapheme width to 2 cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All emoji graphemes are double-width. Foot doesn’t support non-latin scripts. Ergo, this should result in the Right Thing, even though we’re not doing it the Right Way. Note that we’re now breaking cursor synchronization with nearly all applications. But the way I see it, the applications need to be updated. --- vt.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/vt.c b/vt.c index 900ffd9d..5f2b3c0d 100644 --- a/vt.c +++ b/vt.c @@ -766,11 +766,9 @@ action_utf8_print(struct terminal *term, wchar_t wc) int grapheme_width = composed != NULL ? composed->width : base_width; - if (wc == 0xfe0f && grapheme_width < 2) - grapheme_width = 2; - else - grapheme_width += width; - new_cc->width = grapheme_width; + if (wc == 0xfe0f) + width = 2; + new_cc->width = min(max(grapheme_width, width), 2); term->composed_count++; composed_insert(&term->composed, new_cc); From 9817e44c325f5d44f64bfa768107998134ea7f59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Jun 2021 18:00:33 +0200 Subject: [PATCH 15/17] config: add tweak.grapheme-width-method=wcswidth|at-most-2 --- CHANGELOG.md | 3 +++ config.c | 10 ++++++++++ config.h | 1 + doc/foot.ini.5.scd | 18 ++++++++++++++++++ vt.c | 19 ++++++++++++++----- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78016bae..9337652b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ terminfo entries. This should make 24-bit RGB colors work in tmux and neovim, without the need for config hacks or detection heuristics (https://codeberg.org/dnkl/foot/issues/615). +* `[tweak].grapheme-width-method=wcswidth|at-most-2` option to + `foot.ini`. + ### Changed ### Deprecated diff --git a/config.c b/config.c index 372e9f9c..cb407c24 100644 --- a/config.c +++ b/config.c @@ -2255,6 +2255,15 @@ parse_section_tweak( LOG_WARN("tweak: grapheme shaping"); } + else if (strcmp(key, "grapheme-width-method") == 0) { + if (strcmp(value, "at-most-2") == 0) + conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_MAX_2; + else if (strcmp(value, "wcswidth") == 0) + conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_WCSWIDTH; + + LOG_WARN("%s:%d [tweak]: grapheme-width-method=%s", path, lineno, value); + } + else if (strcmp(key, "render-timer") == 0) { if (strcmp(value, "none") == 0) { conf->tweak.render_timer_osd = false; @@ -2823,6 +2832,7 @@ config_load(struct config *conf, const char *conf_path, .fcft_filter = FCFT_SCALING_FILTER_LANCZOS3, .allow_overflowing_double_width_glyphs = true, .grapheme_shaping = false, + .grapheme_width_method = GRAPHEME_WIDTH_MAX_2, .delayed_render_lower_ns = 500000, /* 0.5ms */ .delayed_render_upper_ns = 16666666 / 2, /* half a frame period (60Hz) */ .max_shm_pool_size = 512 * 1024 * 1024, diff --git a/config.h b/config.h index ee427267..57305b22 100644 --- a/config.h +++ b/config.h @@ -246,6 +246,7 @@ struct config { enum fcft_scaling_filter fcft_filter; bool allow_overflowing_double_width_glyphs; bool grapheme_shaping; + enum {GRAPHEME_WIDTH_WCSWIDTH, GRAPHEME_WIDTH_MAX_2} grapheme_width_method; bool render_timer_osd; bool render_timer_log; bool damage_whole_window; diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 2e408b3a..a0829ad7 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -997,8 +997,26 @@ any of these options. but is necessary to not break cursor synchronization with the application running in foot. + See also: *grapheme-width-method*. + Default: _no_ +*grapheme-width-method* + Selects which method to use when calculating the width + (i.e. number of columns) of a grapheme cluster. One of *at-most-2* + and *wcswidth*. + + *wcswidth* simply adds together the individual width of all + codepoints making up the cluster. + + *at-most-2* does the same, but limits the maximum number of + columns to 2. This is more correct, but is likely to break + applications since applications typically use *wcswidth*(3) + internally to calculate the width. This results in cursor + de-synchronization issues. + + Default: _at-most-2_ + *max-shm-pool-size-mb* This option controls the amount of virtual address space used by the pixmap memory to which the terminal screen content is diff --git a/vt.c b/vt.c index 5f2b3c0d..6ef2b782 100644 --- a/vt.c +++ b/vt.c @@ -764,17 +764,26 @@ action_utf8_print(struct terminal *term, wchar_t wc) (wanted_count - 2) * sizeof(new_cc->chars[0])); } - int grapheme_width = composed != NULL ? composed->width : base_width; + const int grapheme_width = + composed != NULL ? composed->width : base_width; - if (wc == 0xfe0f) - width = 2; - new_cc->width = min(max(grapheme_width, width), 2); + switch (term->conf->tweak.grapheme_width_method) { + case GRAPHEME_WIDTH_MAX_2: + if (unlikely(wc == 0xfe0f)) + width = 2; + new_cc->width = min(grapheme_width + width, 2); + break; + + case GRAPHEME_WIDTH_WCSWIDTH: + new_cc->width = grapheme_width + width; + break; + } term->composed_count++; composed_insert(&term->composed, new_cc); wc = CELL_COMB_CHARS_LO + key; - width = grapheme_width; + width = new_cc->width; xassert(wc >= CELL_COMB_CHARS_LO); xassert(wc <= CELL_COMB_CHARS_HI); From 5138f022146e447d8bb0be56a06e9065b35b6489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 1 Jul 2021 08:00:23 +0200 Subject: [PATCH 16/17] config: rename at-most-2 (value for grapheme-width-method) to double-width --- CHANGELOG.md | 2 +- config.c | 6 +++--- config.h | 2 +- doc/foot.ini.5.scd | 8 ++++---- vt.c | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9337652b..0b79b0ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ terminfo entries. This should make 24-bit RGB colors work in tmux and neovim, without the need for config hacks or detection heuristics (https://codeberg.org/dnkl/foot/issues/615). -* `[tweak].grapheme-width-method=wcswidth|at-most-2` option to +* `[tweak].grapheme-width-method=wcswidth|double-width` option to `foot.ini`. diff --git a/config.c b/config.c index cb407c24..e02ca3de 100644 --- a/config.c +++ b/config.c @@ -2256,8 +2256,8 @@ parse_section_tweak( } else if (strcmp(key, "grapheme-width-method") == 0) { - if (strcmp(value, "at-most-2") == 0) - conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_MAX_2; + if (strcmp(value, "double-width") == 0) + conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_DOUBLE; else if (strcmp(value, "wcswidth") == 0) conf->tweak.grapheme_width_method = GRAPHEME_WIDTH_WCSWIDTH; @@ -2832,7 +2832,7 @@ config_load(struct config *conf, const char *conf_path, .fcft_filter = FCFT_SCALING_FILTER_LANCZOS3, .allow_overflowing_double_width_glyphs = true, .grapheme_shaping = false, - .grapheme_width_method = GRAPHEME_WIDTH_MAX_2, + .grapheme_width_method = GRAPHEME_WIDTH_DOUBLE, .delayed_render_lower_ns = 500000, /* 0.5ms */ .delayed_render_upper_ns = 16666666 / 2, /* half a frame period (60Hz) */ .max_shm_pool_size = 512 * 1024 * 1024, diff --git a/config.h b/config.h index 57305b22..2300a0ad 100644 --- a/config.h +++ b/config.h @@ -246,7 +246,7 @@ struct config { enum fcft_scaling_filter fcft_filter; bool allow_overflowing_double_width_glyphs; bool grapheme_shaping; - enum {GRAPHEME_WIDTH_WCSWIDTH, GRAPHEME_WIDTH_MAX_2} grapheme_width_method; + enum {GRAPHEME_WIDTH_WCSWIDTH, GRAPHEME_WIDTH_DOUBLE} grapheme_width_method; bool render_timer_osd; bool render_timer_log; bool damage_whole_window; diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index a0829ad7..684497ca 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -1003,19 +1003,19 @@ any of these options. *grapheme-width-method* Selects which method to use when calculating the width - (i.e. number of columns) of a grapheme cluster. One of *at-most-2* - and *wcswidth*. + (i.e. number of columns) of a grapheme cluster. One of + *double-width* and *wcswidth*. *wcswidth* simply adds together the individual width of all codepoints making up the cluster. - *at-most-2* does the same, but limits the maximum number of + *double-width* does the same, but limits the maximum number of columns to 2. This is more correct, but is likely to break applications since applications typically use *wcswidth*(3) internally to calculate the width. This results in cursor de-synchronization issues. - Default: _at-most-2_ + Default: _double-width_ *max-shm-pool-size-mb* This option controls the amount of virtual address space used by diff --git a/vt.c b/vt.c index 6ef2b782..b9a4f4b0 100644 --- a/vt.c +++ b/vt.c @@ -768,7 +768,7 @@ action_utf8_print(struct terminal *term, wchar_t wc) composed != NULL ? composed->width : base_width; switch (term->conf->tweak.grapheme_width_method) { - case GRAPHEME_WIDTH_MAX_2: + case GRAPHEME_WIDTH_DOUBLE: if (unlikely(wc == 0xfe0f)) width = 2; new_cc->width = min(grapheme_width + width, 2); From ab8f9afa909395c691013e3ee99ceb6f01003063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 1 Jul 2021 20:13:03 +0200 Subject: [PATCH 17/17] =?UTF-8?q?changelog:=20move=20grapheme=20cluster=20?= =?UTF-8?q?width=20entry=20to=20=E2=80=98changed=E2=80=99,=20and=20rewrite?= =?UTF-8?q?=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b79b0ac..f38d3f18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,11 +36,16 @@ terminfo entries. This should make 24-bit RGB colors work in tmux and neovim, without the need for config hacks or detection heuristics (https://codeberg.org/dnkl/foot/issues/615). -* `[tweak].grapheme-width-method=wcswidth|double-width` option to - `foot.ini`. ### Changed + +* Grapheme cluster width is now limited to two cells by default. This + may cause cursor synchronization issues with many applications. You + can set `[tweak].grapheme-width-method=wcswidth` to revert to the + behavior from foot-1.8.0. + + ### Deprecated ### Removed ### Fixed