Skip to content
This repository has been archived by the owner on Jan 10, 2024. It is now read-only.

Support ANSI SGR crossedout and conceal #65

Closed
wants to merge 114 commits into from

Conversation

alanswanson
Copy link

Vim was causing lots of (Unimplemented) attribute: 29 to be logged at start up and every PGUP/PGDOWN key press so here's a fix by supporting crossed-out and also conceal since it was easy.

Tested with 7x13 fixed font to pixel match what xterm uses with y - ((3 * FontAscent(screen)) / 8) versus (srcGlyphPixels.y - 1) * 5 / 8). I presume your preferred 9x18 fixed font should be close but didn't test. Not sure if there is an equivalent to FontAscent in zutty.

(Also locally patched to disable logging for (Unimplemented) Stop blinking cursor too.)

The conceal part could be used for the unimplemented blinking too but not sure where to put a timer, codewise and classwise, that updates a blink_active variable to match xterm 600ms on and 300ms off approximately.

if (conceal == 1u || (blinking == 1u && blink_active)) { fgColor = bgColor; } }

Tom Szilagyi added 30 commits May 2, 2020 21:54
This is much more flexible than using a texture to contain the text
data and all attributes. We can map the storage backing the buffer
and access it directly.
Tom Szilagyi and others added 26 commits March 20, 2021 19:12
This targets palette color query OSC 4;* as well as dynamic color
queries OSC 10, 11, 12, 17, 19. Only queries are implemented, setting
colors in this way is not supported (silently discarded).

Closes #22
This ensures that we generate the correct encoding for the Return key
in the presence of modifiers; the same standard encoding is used with
both modifyOtherKeys = 1 and 2: CSI 27 ; <MC> ; 13 ~
This avoids an infinite loop of ConfigureNotify events in case the WM
sends such an event on every call to XSetWMNormalHints, even if the
reported geometry has not changed.

Closes #38
The font character is loaded with FT_LOAD_RENDER right before this, so
an explicit render call should not be needed. Further, recent FreeType
(2.11.0) seems to return an error (FT_Err_Cannot_Render_Glyph) in this
case, which is probably a bug in FreeType, but is hereby worked around.

See #6 for further info.
Some Vterm methods are used in response to direct input characters
(such as a newline, carriage return or tab) while also called from
other places as building blocks for handling certain escape sequences.
We need to distinguish between these call scenarios to set/maintain
the correct value of curPosViaCharPlacement, which shall be true if
and only if the cursor has arrived at its current position through
character placement (placeGraphicChar) and not via any other kind of
Vterm manipulation.

The distinction is subtle and is probably only observable in
pathological (but nevertheless real-world) scenarios, such as cat'ing
random binary files into the terminal.

Another subtle change is that we drop our ENQ (0x05/Enquiry) response.
This is generally an anachronism, but more specifically, it causes
problematic behaviour in the same real-world case of cat'ing large
binary files. Since the binary will inevitably contain ENQ bytes with
some random frequency, Zutty would emit its response that nobody will
drain. This output eventually fills up the outbound pipe, blocking
Zutty in a writePty().  The behaviour is technically correct, but the
user experience (Zutty stopped responding) is "highly unfortunate".

Closes #47
Add guard for illegal condition of being in the "invisible" off-by-one
row before trying to insert a character. In such a case, unless we
have arrived there via a directly preceding previous character
placement and auto-wrap is on, the new character will be discarded.
Also add guard for running csi_ED and csi_EL with the cursor in this
off-screen column (we need posX + 1 to evaluate to no more than nCols).

This condition does not normally occur, but can be evoked by
pathological (but nevertheless real-world) sequences, such as cat'ing
random binary data into the terminal.

Adding the guards substitutes the relevant parts of the previous commit
37d3781, which are now reverted in exchange for a more relevant fix.

Another subtle change is that we drop our ENQ (0x05/Enquiry) response.
This is generally an anachronism, but more specifically, it causes
problematic behaviour in the same real-world case of cat'ing large
binary files. Since the binary will inevitably contain ENQ bytes with
some random frequency, Zutty would emit its response that nobody will
drain. This output eventually fills up the outbound pipe, blocking
Zutty in a writePty().  The behaviour is technically correct, but the
user experience (Zutty stopped responding) is "highly unfortunate".

Closes #47
Closes #48
Closes #51
This ability is inherent in the way the existing selection adjustment
mechanism works, but was blocked by a (needless) guard.

By the same mechanism, a triple-click selects the complete line.

Closes #49
The issue was the conflation of the state of "empty selection" (with
identical start and end points, that can still be snapped to a word or
line, depending on snapTo) with "no selection". The latter is now
signified by out-of-limits coords - Point is initialized to (-1, -1)
instead of (0, 0), and this state discriminated for via Point::null ().

Closes #52
The code was slightly unclear on whether the selection rect contains
the 'raw' coords (closely reflecting the mouse actions) or the bounds
after snap-to word/line has taken place. This led to faulty behaviour
as exemplified by #55, due to having used the snapped coords where the
raw coords would have been correct.

Fix this by making Frame own the snapTo setting, and by only storing
the 'raw' selection rect and treating the snapped coords as ephemeral
(computed on demand). In practice, the snap-to operation is performed
the same number of times, so this does not even impact performance.

Closes #55
Extend test suite with scripts mentioned in #57, plus verify the
output of wraptest (https://github.com/mattiase/wraptest) in Zutty.
This output is (as of the present commit) perfectly identical to the
output of XTerm.

As a bonus, add support for ED (Erase in Display) with arg 3, which
is an XTerm extension to erase the whole display including the
scrollback buffer.

Closes #57
- Add -dwfont option to specify the name of the double-width font.
  The default setting '18x18ja' matches the default main font '9x18',
  otherwise a font with matching geometry must be specified.

- If the double-width font is not present or cannot be loaded (e.g.,
  because its geometry does not match the main font), double-width
  characters will obviously not appear on screen, but spacing will
  still be correct and copy-pasting them will still work (as we have
  the unicode points, we just cannot render them).

- Unicode display correctness has marginally improved in a few cases:
  U+0080 was drawn as a missing glyph (hinting the user that a better
  font would perhaps make it visible), whereas it is really a control
  character with no valid visual, hence the Unicode Replacement Char
  should be shown. The opposite change happened to U+10000 which is
  in fact a valid (if uncommon) character, but was rendered as RC.
  U+D7FF is now also correctly shown as a RC (as it belongs to a
  reserved range). Finally, the same correction was applied to the
  non-characters in the range U+FDD0..U+FDEF.

- As a bonus, the Fontpack now releases all fonts from host memory
  (previously held for the rest of the program's life) once all font
  data is loaded into GL. This frees several megabytes of host RAM
  (the issue became more pronounced with CJK fonts having tens of
  thousands of glyphs).
On certain non-Linux systems (e.g., OpenBSD), poll () sets POLLHUP in
.revents to signal EOF, and this can happen before the first byte of
data arrives from the pty slave (written by the shell). That is, the
pty fd appears to have been closed from the other end, but this is
only a temporary condition.

The current (lack of) handling this leads, on certain systems, to
sporadic "startup errors" of Zutty wherein the program's window
appears and immediately disappears. To avoid this, unify our handlers
of POLLIN and POLLHUP, and exit on an EOF (which is what we get on a
read () after a POLLHUP) only after a successful read has happened
before, else keep looping.

Also, for the sake of portability, honor EINTR and retry the poll ()
instead of giving up.
The issue was caused by a conflation of numpad keysyms received in
different NumLock states. The "NumLock off" keysyms (e.g., XK_KP_Up)
must be used to map keys in DECKPAM (application mode), rather than
the "NumLock on" keysyms (e.g., XK_KP_8).

As a further correction, ensure that keys that are reported with the
same keysyms regardless of NumLock state (e.g., XK_KP_Divide) will
correctly map to their literals (e.g., '/') when NumLock is on, even
in application mode.

Finally, avoid emitting a NUL on receiving the NumLock keypress itself.

These changes make Zutty behaviour equivalent to XTerm configured for
VT220-style function keys.

Closes #60
These have somehow slipped through earlier rounds of scrutiny, and
produced incorrect sequences with modifyOtherKeys=2. Now corrected
with documentation and test coverage added for them.

Closes #61
Still not perfect: depending on font and size, artefacts similar to
those described in the ticket might be observable. I believe these
are due to rounding errors (discarded subpixel amounts).

As stated, this is probably not the final word on the issue, but a
definitive improvement worth committing.

Closes #62
Xterm uses "y - ((3 * FontAscent(screen)) / 8)" and this
matches on 7x13 fixed font.
Just sets foreground colour to match background colour. Text
is still selectable and will be visible when cursor is over
glyph.

Conceal is not security feature otherwise why even write to
screen? Most other terminal emulators (that support conceal)
do the same as here and only xterm prevents copy selecting
by replacing characters with spaces.
@alanswanson
Copy link
Author

Updated with gpg commit signature. I hadn't seen the merging block for signed commits only for some reason.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants