Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emulation: bold off broken and dim not implemented? #1042

Closed
theflyingape opened this issue Oct 9, 2017 · 7 comments
Closed

Emulation: bold off broken and dim not implemented? #1042

theflyingape opened this issue Oct 9, 2017 · 7 comments
Assignees
Labels
type/enhancement Features or improvements to existing features
Milestone

Comments

@theflyingape
Copy link
Contributor

Shutting off (20 + attribute) bold is not working, although others implemented (uline and reverse) do, i.e.,

echo -e '\x1B[33m yellow \x1B[1m bold \x1B[21m normal \x1B[m off'
echo -e '\x1B[33m yellow \x1B[4m uline \x1B[24m normal \x1B[m off'
echo -e '\x1B[33m yellow \x1B[7m reverse \x1B[27m normal \x1B[m off'

Oddly enough, changing 21 (bold off) to 22 (dim off) works:
echo -e '\x1B[33m yellow \x1B[1m bold \x1B[22m normal \x1B[m off'

... but bold -> dim enable keeps bold on -- I thought bold/dim are implemented as mutually exclusive, unlike underline, blink, and reverse that can be added or taken away with either bold or dim? At the very least if dim is not implemented, it should shutoff bold, but it does not:
echo -e '\x1B[33m yellow \x1B[1m bold \x1B[2m dim \x1B[m off'

Two related questions:

  1. I get blink (at least for now ;) not implemented, but how about dim and italic?
  2. Can a custom color palette be constructed? The one in v3 branch (below) is more palatable to me.

Details

  • Browser and browser version: Firefox 56 (64-bit)
  • OS version: Fedora 26 (linux)
  • xterm.js version: 2.9.2 and latest v3 branch

Steps to reproduce

$ cat bin/ansi-test.sh
#!/bin/sh

alias print='echo -en'
alias println='echo -e'
csi="\\x1B["
color=( black red green yellow blue magenta cyan white )
attr=( reset bright dim italic uline blink 6 reverse )

for bg in `seq 0 7`; do
        print "(${csi}m${bg}) BG ${csi}0;1;$((30+${bg}))m ${color[${bg}]}${csi}m"
        for fg in `seq 0 7`; do
                print "\n    FG ${csi}0;1;$((30+${fg}))m ${color[${fg}]}${csi}m\t"
                print "${csi}$((40+${bg}))m"
                print "${csi}$((30+${fg}))m"
                for ca in `seq 1 7`; do
                        print "${csi}${ca}m  ${attr[${ca}]}  ${csi}$((20+${ca}))m"
                done
                print "${csi}m"
        done
        println "${csi}m"
done

Showing Terminology - xterm.js - v3 branch:
terminology xterm v3

@mofux
Copy link
Contributor

mofux commented Oct 9, 2017

@theflyingape I don't think bold/dim are mutually exclusive, at least not in Terminal.app:

screen shot 2017-10-09 at 15 58 51

@mofux
Copy link
Contributor

mofux commented Oct 9, 2017

I can confirm dim support is not available, and looking into master, probably never has been?

@theflyingape
Copy link
Contributor Author

theflyingape commented Oct 9, 2017

Thanks, I do not understand your reference to Terminal.app, I was running the demo/app as pictured.
Does bold have to change font style, or can it be configured to only alter color?
What you show here is closer to what is needed: I see your dim changed the color to a darker yellow -- yay, although I cannot reproduce that in demo/app. However, it kept the font-style as bold whereas I'd expect it to back to normal -- or a lighter weight to be consistent with bold?

@mofux
Copy link
Contributor

mofux commented Oct 9, 2017

I am referring to the behaviour of clearing the bold style when setting dim. Terminal.app does not clear the bold style on dim. xterm (v3 branch) behaves exactly the same, but it does not support dim yet, which is why the text is not dimmed:

xterm.js (v3)
screen shot 2017-10-09 at 16 17 55

@theflyingape
Copy link
Contributor Author

I see your point clearly now. And I now see InputHandler.ts has a p === 22 listed as "not bold".
Until dim is supported, can that be changed to (p === 21 || p === 22) to allow for the correct 21 code to be consumed while maintaining backward compatibility?

@mofux
Copy link
Contributor

mofux commented Oct 9, 2017

I'll try to create a PR to enable dim support.

@mofux mofux added the type/enhancement Features or improvements to existing features label Oct 9, 2017
@mofux mofux self-assigned this Oct 9, 2017
@mofux mofux added this to the 3.0.0 milestone Oct 9, 2017
@Tyriar
Copy link
Member

Tyriar commented Oct 9, 2017

Related: #580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

3 participants