fix: Display offset for filenames with spaces

[guillaumeboehm]

Reopening #548 in different issue.

[PThorpe92]

I think if you delete that #[rustfmt::skip] that was in the middle of that function. and run rustfmt, it might fix the build. that's what I was trying to do when i accidentally closed ur last PR.

(that was obviously already there.. not your fault, I was just trying to clean it up and it so happened to be on your fork because you're touching this part of the code)

[guillaumeboehm]

I don't really understand what the checks are supposed to check ? Is there a way for me to check on my machine directly without having to push to launch a job ?

EDIT: Oh I'm actually just stupid I didn't realize rustfmt was a command... Testing that

[guillaumeboehm]

So much for me trying to format the code nicely myself xD

[PThorpe92]

After further review I believe we also need to check for

if file.name.contains(' ') || file.name.contains('\'') { 2 } else { 0 }

as a file name with a quote in it will cause it to be quoted as well. This is found in the check in src/options/escape.rs

    let needs_quotes = string.contains(' ') || string.contains('\'');

After that LGTM 👍

[guillaumeboehm]

Also fixes #515 I believe

[cafkafk]

cargo run -- --blocksize -l --hyperlink --grid --git-repos -x --time-style "+%Y, %Y, %d%d%d - %a %u" --git --header --icons -Z -@ -o

This bugfix targets this bug right? It's still present as far as I can tell

[cafkafk]

sorry I can't be of more help with this currently I'm a bit busy irl

[guillaumeboehm]

I didn't realize there was a different code for detailed outputs, it should be good now if I didn't mess up the formatting yet again.

[PThorpe92]

sorry I can't be of more help with this currently I'm a bit busy irl

No worries at all ❤️

cargo run -- --blocksize -l --hyperlink --grid --git-repos -x --time-style "+%Y, %Y, %d%d%d - %a %u" --git --header --icons -Z -@ -o

This bugfix targets this bug right? It's still present as far as I can tell

Wow I can't believe I didn't see that and frankly I'm surprised even the tests didn't catch that one..

Ok tested locally this looks good. Can I get another @eza-community/eza-commiters review on this?

[PThorpe92]

LGTM Thanks 👍

[ariasuni]

Wow I can't believe I didn't see that and frankly I'm surprised even the tests didn't catch that one..

Yeah we should probably add a test for that

[PThorpe92]

Yeah we should probably add a test for that

Indeed. The current tests all have filenames without spaces or quotes, and none of them are longer than a few characters. Definitely not ideal

[cafkafk]

This seems to fix it, and integration tests pass, LGTM thanks for fixing this

[aarondill]

@cafkafk @guillaumeboehm this only fixes the issue when quotes are being displayed. After this patch, using --no-quotes now results in files being offset to the left:

[guillaumeboehm]

@cafkafk @guillaumeboehm this only fixes the issue when quotes are being displayed. After this patch, using --no-quotes now results in files being offset to the left:

Oh I didn't see that option... I'll fix this when I get home.

[PThorpe92]

I got a PR ready unless you'd like to submit one @guillaumeboehm

[guillaumeboehm]

I got a PR ready unless you'd like to submit one @guillaumeboehm

No it's all good if you have one ! Thanks ^^