coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

ls: Unexpected space before stdout in TTY output

Open RenjiSann opened this issue 9 months ago • 8 comments

I was fixing TODOs in the testsuite to check TTY outputs.

And I realized that there is a seemingly unexpected character (presumably a space) that gets printed.

❯ /bin/ls "$(echo -e 'a\nb')" --hide-control-chars  
'a'$'\n''b'

❯ target/debug/ls "$(echo -e 'a\nb')" --hide-control-chars
 'a'$'\n''b'
^
Unexpected space

The default escaping strategy when outputing to a TTY is shell-escape. Strangely, the bug does not appear when output is not a TTY but with --quoting-style=shell-escape set, as already checked by the testsuite

RenjiSann avatar May 08 '24 21:05 RenjiSann

Well maybe it is not exactly a space. I have written this test:

  scene
      .ucmd()
      .arg("--hide-control-chars")
      .arg("one\ntwo")
      .terminal_simulation(true)
      .succeeds()
      .stdout_only(" 'one'$'\\n''two'\n");

and this still fails, but the diff message is broken:

assertion failed: `(left == right)`

Diff < left / right > :
  'one'$'\n''two'
 

I'll investigate more to find what is this character and try to find where it comes from.

RenjiSann avatar May 08 '24 21:05 RenjiSann

I modified the assertion code to dump the bytes of expected/got stdouts on failure, and I ended up with this:

---- test_ls::test_ls_quoting_and_color stdout ----
touch: /tmp/.tmpyfPDjs/one two
run: /home/renji/projects/coreutils/target/debug/coreutils ls --color one two
run: /home/renji/projects/coreutils/target/debug/coreutils ls --color one two
EXPECTED: [32, 39, 111, 110, 101, 32, 116, 119, 111, 39, 10]
GOT     : [32, 39, 111, 110, 101, 32, 116, 119, 111, 39, 13, 10]
thread 'test_ls::test_ls_quoting_and_color' panicked at tests/by-util/test_ls.rs:2905:10:
explicit panic

So it appears that the prefix character is indeed a 0x20 space, but the code also includes a trailing carriage return \r (0x0D) before the linefeed \n.

I still don't have a clue for why is this happening though.

EDIT:

I keep finding things ! I don't know if it is specific to my computer, but eventhough I don't know why, /bin/ls does include a \r as well, so uu-ls must be right to do so.

For reference:

❯ script -q -c '/bin/ls "$(echo "a\nb")"' | xxd
00000000: 2761 2724 275c 6e27 2762 270d 0a         'a'$'\n''b'..

❯ script -q -c 'target/debug/ls "$(echo "a\nb")"' | xxd
00000000: 2027 6127 2427 5c6e 2727 6227 0d0a        'a'$'\n''b'..

Now what's left is to actually understand WHY a space is inserted here.

RenjiSann avatar May 08 '24 22:05 RenjiSann

Found: this is where the space is added.

No searching why is the behavior only triggered by TTY

RenjiSann avatar May 08 '24 23:05 RenjiSann

I removed the "adding space if quoted and starts with '" logic here and it didn't break any test.

I am not sure what it is made for, but so far, removing it fixes my problem.

@tertsdiepraam I see you changed this code in 3346b4147aa7c9dd5e5ad822ac927407010eb8de, maybe you'll have some insights.

RenjiSann avatar May 08 '24 23:05 RenjiSann

For anyone else, link to that commit: https://github.com/uutils/coreutils/commit/3346b4147aa7c9dd5e5ad822ac927407010eb8de

Seems like I made a mistake? The logic should be that there's a space if the quoting style does not add a quote, but I seem to have flipped that behaviour? That's not good :)

Negating the condition should fix it. But, we also need to think about when it's quoted with " maybe. We might need to come up with a more robust check. Do you want to give that a shot?

tertsdiepraam avatar May 11 '24 21:05 tertsdiepraam

For anyone else, link to that commit: 3346b41

Seems like I made a mistake? The logic should be that there's a space if the quoting style does not add a quote, but I seem to have flipped that behaviour? That's not good :)

Negating the condition should fix it. But, we also need to think about when it's quoted with " maybe. We might need to come up with a more robust check. Do you want to give that a shot?

I can, but I don't have all the context. I don't know in what case a space is required in the first place. Would you have an example to share, or some doc that talk about this behavior ?

RenjiSann avatar May 13 '24 17:05 RenjiSann

It's just an alignment thing. Here's an example using GNU:

> ls
'bla bla'   Cargo.lock   CODE_OF_CONDUCT.md   deny.toml        docs   GNUmakefile   Makefile        oranda.json   renovate.json   target   util
 build.rs   Cargo.toml   CONTRIBUTING.md      DEVELOPMENT.md   fuzz   LICENSE       Makefile.toml   README.md     src             tests

Note how all the files have a space to compensate for not having quotes and that 'bla bla' doesn't have a space.

tertsdiepraam avatar May 13 '24 17:05 tertsdiepraam

Oh, I think I get it.

I'll give it a try !

RenjiSann avatar May 13 '24 22:05 RenjiSann