serenity
serenity copied to clipboard
SerenityOS
LibVT: Handle concealed ANSI escape codes
This implements concealed and not concealed ANSI escape sequences: 8 and 28 respectively, as per https://en.wikipedia.org/wiki/ANSI_escape_code
Hello!
One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.
What happens in other terminals when you copy concealed text? Does it make it to the clipboard?
This is a great question.
From the linked Wikipedia article, there's this code:
#include <stdio.h>
int main(void)
{
int i, j, n;
for (i = 0; i < 11; i++) {
for (j = 0; j < 10; j++) {
n = 10 * i + j;
if (n > 108) break;
printf("\033[%dm %3d\033[m", n, n);
}
printf("\n");
}
return 0;
}
I compiled it on Fedora, in Gnome terminal:
The result:
AlgorithmWolf@neon:~/Downloads$ ./demo
0 1 2 3 4 5 6 7 8 9
10 11 12 13 14 15 16 17 18 19
20 21 22 23 24 25 26 27 28 29
30 31 32 33 34 35 36 37 38 39
40 41 42 43 44 45 46 47 48 49
50 51 52 53 54 55 56 57 58 59
60 61 62 63 64 65 66 67 68 69
70 71 72 73 74 75 76 77 78 79
80 81 82 83 84 85 86 87 88 89
90 91 92 93 94 95 96 97 98 99
100 101 102 103 104 105 106 107 108
The way I'm handling this means it is equivalent to whitespace, so the text will indeed not be copied to the clipboard.
Let me dive into how to handle this differently.
In st from suckless: https://st.suckless.org/
if (base.mode & ATTR_BLINK && win.mode & MODE_BLINK)
fg = bg;
if (base.mode & ATTR_INVISIBLE)
fg = bg;
They set the bg and fg to the same colour. I guess that works :man_shrugging:
The question then is...
What if the concealed text had some foreground and background colour? Do we overwrite the foreground with the background colour for as long as the conceal flag is set?
"When painting a concealed cell, paint it with bg as foreground color instead of with the current foreground color" seems like an elegant approach to me! Do you think that could work for us?
They set the bg and fg to the same colour. I guess that works 🤷♂️
That depends on what you mean by colour. I've noticed that black text on black background is visible with the default settings.
They set the bg and fg to the same colour. I guess that works 🤷♂️
That depends on what you mean by colour. I've noticed that black text on black background is visible with the default settings.
In serenity? Do you have a screenshot?
When painting a concealed cell, paint it with bg as foreground color
Err...why paint it at all? Just paint the background and move on?
Tested on commit 99b0181
That looks like an issue with the colour scheme rather than inherent to the ANSI escape codes or the way concealed text is painted. Here's your code in st: works as expected.
That looks like an issue with the colour scheme rather than inherent to the ANSI escape codes or the way concealed text is painted. Here's your code in st: works as expected.
Yes, I know. It would however be an issue on SerenityOS terminal if conceal is implemented this way. Could easily be fixed though.
@nico It seems the text is copied fine to the clipboard with my method. Do you still prefer I implement paint-character-as-background-colour even though the colour schemes are a bit broken right now as per #24961 ?
@algorithmwolf fwiw it looks like your git config user.email used for this patch is not linked to your GitHub account. if you do link it, then it will be easier to attribute to you from the commit log.
@ADKaster thank you for bringing that up. I noticed it and fixed it recently. I will double check in my next merge. Thanks again!
(you can add a line to https://github.com/SerenityOS/serenity/blob/master/.mailmap to map that email to your GitHub-associated emails. Or you can add that email to your GitHub profile emails.)
I would have to re-do this commit because the e-mail I used for this one is one I no longer use. But really, not that big of a deal. I'll add myself to .mailmap