SS2 and SS3 lock the charset #580

Closed
opened 5 months ago by dnkl · 4 comments
dnkl commented 5 months ago
Owner

SS2 and SS3 should only change the charset for the next character.

Foot does not switch back to the original charset.

SS2 and SS3 should only change the charset for the next character. Foot does not switch back to the original charset.
dnkl added the
bug
label 5 months ago
Collaborator

I was thinking of something along the lines of:

diff --git a/terminal.c b/terminal.c
index 3d6c20d..5ba7b85 100644
--- a/terminal.c
+++ b/terminal.c
@@ -2839,6 +2839,14 @@ ascii_printer_fast(struct terminal *term, wchar_t wc)
         xassert(!term->grid->cursor.lcf);
 }
 
+static void
+ascii_printer_single_shift(struct terminal *term, wchar_t wc)
+{
+    ascii_printer_generic(term, wc);
+    term->charsets.selected = term->charsets.saved;
+    term_update_ascii_printer(term);
+}
+
 void
 term_update_ascii_printer(struct terminal *term)
 {
@@ -2860,6 +2868,14 @@ term_update_ascii_printer(struct terminal *term)
     term->ascii_printer = new_printer;
 }
 
+void
+term_set_single_shift_ascii_printer(struct terminal *term, int selected)
+{
+    term->charsets.saved = term->charsets.selected;
+    term->charsets.selected = selected;
+    term->ascii_printer = &ascii_printer_single_shift;
+}
+
 enum term_surface
 term_surface_kind(const struct terminal *term, const struct wl_surface *surface)
 {
diff --git a/terminal.h b/terminal.h
index 1721a4c..a54185e 100644
--- a/terminal.h
+++ b/terminal.h
@@ -192,6 +192,7 @@ enum charset { CHARSET_ASCII, CHARSET_GRAPHIC };
 
 struct charsets {
     int selected;
+    int saved;
     enum charset set[4]; /* G0-G3 */
 };
 
@@ -605,6 +606,7 @@ bool term_shutdown(struct terminal *term);
 int term_destroy(struct terminal *term);
 
 void term_update_ascii_printer(struct terminal *term);
+void term_set_single_shift_ascii_printer(struct terminal *term, int selected);
 
 void term_reset(struct terminal *term, bool hard);
 bool term_to_slave(struct terminal *term, const void *data, size_t len);
diff --git a/vt.c b/vt.c
index d0d215f..d813fec 100644
--- a/vt.c
+++ b/vt.c
@@ -414,14 +414,12 @@ action_esc_dispatch(struct terminal *term, uint8_t final)
 
         case 'N':
             /* SS2 - Single Shift 2 */
-            term->charsets.selected = 2; /* G2 */
-            term_update_ascii_printer(term);
+            term_set_single_shift_ascii_printer(term, 2); /* G2 */
             break;
 
         case 'O':
             /* SS3 - Single Shift 3 */
-            term->charsets.selected = 3; /* G3 */
-            term_update_ascii_printer(term);
+            term_set_single_shift_ascii_printer(term, 3); /* G3 */
             break;
 
         case '\\':

However, this allows other state changes in-between the single shift and the next printable character. I'm not sure if that could lead to undesirable results, although it doesn't seem like a sane thing for a client to do.

I was thinking of something along the lines of: ```diff diff --git a/terminal.c b/terminal.c index 3d6c20d..5ba7b85 100644 --- a/terminal.c +++ b/terminal.c @@ -2839,6 +2839,14 @@ ascii_printer_fast(struct terminal *term, wchar_t wc) xassert(!term->grid->cursor.lcf); } +static void +ascii_printer_single_shift(struct terminal *term, wchar_t wc) +{ + ascii_printer_generic(term, wc); + term->charsets.selected = term->charsets.saved; + term_update_ascii_printer(term); +} + void term_update_ascii_printer(struct terminal *term) { @@ -2860,6 +2868,14 @@ term_update_ascii_printer(struct terminal *term) term->ascii_printer = new_printer; } +void +term_set_single_shift_ascii_printer(struct terminal *term, int selected) +{ + term->charsets.saved = term->charsets.selected; + term->charsets.selected = selected; + term->ascii_printer = &ascii_printer_single_shift; +} + enum term_surface term_surface_kind(const struct terminal *term, const struct wl_surface *surface) { diff --git a/terminal.h b/terminal.h index 1721a4c..a54185e 100644 --- a/terminal.h +++ b/terminal.h @@ -192,6 +192,7 @@ enum charset { CHARSET_ASCII, CHARSET_GRAPHIC }; struct charsets { int selected; + int saved; enum charset set[4]; /* G0-G3 */ }; @@ -605,6 +606,7 @@ bool term_shutdown(struct terminal *term); int term_destroy(struct terminal *term); void term_update_ascii_printer(struct terminal *term); +void term_set_single_shift_ascii_printer(struct terminal *term, int selected); void term_reset(struct terminal *term, bool hard); bool term_to_slave(struct terminal *term, const void *data, size_t len); diff --git a/vt.c b/vt.c index d0d215f..d813fec 100644 --- a/vt.c +++ b/vt.c @@ -414,14 +414,12 @@ action_esc_dispatch(struct terminal *term, uint8_t final) case 'N': /* SS2 - Single Shift 2 */ - term->charsets.selected = 2; /* G2 */ - term_update_ascii_printer(term); + term_set_single_shift_ascii_printer(term, 2); /* G2 */ break; case 'O': /* SS3 - Single Shift 3 */ - term->charsets.selected = 3; /* G3 */ - term_update_ascii_printer(term); + term_set_single_shift_ascii_printer(term, 3); /* G3 */ break; case '\\': ``` However, this allows other state changes in-between the single shift and the next printable character. I'm not sure if that could lead to undesirable results, although it doesn't seem like a sane thing for a client to do.
Collaborator

For reference:

ECMA-48 §8.3.142:

The use of SS3 is defined in Standard ECMA-35.

ECMA-35 §9.4:

When a single-shift function SS2 or SS3 invokes a character from a 94-character or a 94ⁿ-character set the immediately following one or n bit combinations respectively shall be in the range from 02/01 to 07/14.

("02/01 to 07/14" == 0x21 ... 0x7E)

For reference: ECMA-48 §8.3.142: > The use of SS3 is defined in Standard ECMA-35. ECMA-35 §9.4: > When a single-shift function SS2 or SS3 invokes a character from a 94-character or a 94ⁿ-character set the immediately following one or n bit combinations respectively **shall** be in the range from 02/01 to 07/14. ("02/01 to 07/14" == `0x21 ... 0x7E`)
Poster
Owner

So if it's not followed by a character in the range 0x21 ... 0xfe, the behavior is undefined?

That would mean your suggested solution works. I like it, so I say we go with that.

So if it's **not** followed by a character in the range `0x21 ... 0xfe`, the behavior is undefined? That would mean your suggested solution works. I like it, so I say we go with that.
Collaborator

0x21 ... 0xfe,

0x21 ... 0x7e.

the behavior is undefined?

That seems to be the implication from what I can tell, although I think it's closer to "implementation defined".

That would mean your suggested solution works.

Yeah, I think it should work well enough in practice. We can always revisit the edge cases later if need be.

... I say we go with that.

I've pushed the patch to #582; along with the locking shifts.

> 0x21 ... 0xfe, 0x21 ... 0x7e. > the behavior is undefined? That seems to be the implication from what I can tell, although I think it's closer to "implementation defined". > That would mean your suggested solution works. Yeah, I think it should work well enough in practice. We can always revisit the edge cases later if need be. > ... I say we go with that. I've pushed the patch to #582; along with the locking shifts.
craigbarnes closed this issue 5 months ago
craigbarnes changed title from SS2 and SS2 lock the charset to SS2 and SS3 lock the charset 4 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.