skip extension if no ext found #425

zwpaper · 2020-10-02T08:42:28Z

I also found the output is a little bit different like ls, as lsd will display the UTF-8 in file names, It seems like work as expect IMO. If necessary we can discuss this in a new issue. @meain

ls:

[root@b78708e2fc75 tmp]# \ls
'5a35.ico'$'\247\375''='$'\356\324\373\206\362\300''^m'$'\330'

lsd:

[root@b78708e2fc75 tmp]# lsd
 5a35.ico��=������^m�

FYI:
the utf8 code in the filename is reserved for utf-16(https://en.wikipedia.org/wiki/UTF-16#U+D800_to_U+DFFF), and rust will check this by default.

Following the rust way, we display the reserved chars as �

codecov-commenter · 2020-10-02T08:44:44Z

Codecov Report

Merging #425 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   77.74%   77.77%   +0.03%     
==========================================
  Files          34       34              
  Lines        3329     3325       -4     
==========================================
- Hits         2588     2586       -2     
+ Misses        741      739       -2

Impacted Files	Coverage Δ
src/meta/name.rs	`93.80% <100.00%> (+0.76%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4991436...2363960. Read the comment docs.

zwpaper · 2020-10-02T08:47:43Z

BTW, I have tried to find a way to escape the invalid utf-8 code in the filename, but no luck.

path.file_name() returns an Option<&OsStr>, If invalid utf-8 contained, it was None, I can not find a way to get the name, can u give me some tips for getting it? even we did not need to print it in lsd, just out of curiosity.

Peltoche · 2020-10-02T09:09:10Z

Hello @zwpaper, thanks for this PR.

Can you add a test with the filename from the issue? This would allow us to confirm that the code work as expected and to avoid any future regression.

zwpaper · 2020-10-02T09:14:55Z

Hi @Peltoche , glad that you have time for lsd again 😃

I have tried to add the test for this, but this happened only if there is an invalid utf-8 char,
and rust keeps preventing me create the invalid utf-8 char.

unicode escape must not be a surrogate always happens after invalid utf-8 chars added

0jdxt · 2020-10-06T20:32:32Z

tests/integration.rs

+    Command::new("python3")
+        .arg("-c")
+        .arg(r#"import pathlib; pathlib.Path('/tmp/bad.extension/bad.extension\udca7\udcfd').touch()"#)
+        .output()
+        .expect("failed to create file");


Hey man it may be best to avoid having python as a dependency, this can be done in sh with

$ sh -c "touch $(printf 'bad.extension\xdca7\xdcfd')"

Suggested change

Command::new("python3")

.arg("-c")

.arg(r#"import pathlib; pathlib.Path('/tmp/bad.extension/bad.extension\udca7\udcfd').touch()"#)

.output()

.expect("failed to create file");

Command::new("sh")

.arg("-c")

.arg("touch $(printf '/tmp/bad.extension/bad.extension\xdca7\xdcfd')")

.output()

.expect("failed to create file");

I don't want the python dep either, but I can not find a way to create the invalid utf-8 name even use your code.🤣

I have tried the solution like this, The output of the sh -c touch is iliteral \xdca7, not an utf-8 code.

invalid utf-8 code will be treated as error if using double quote.

---- test_bad_utf_8_extension stdout ---- thread 'test_bad_utf_8_extension' panicked at 'Unexpected stdout, failed var.is_match(bad.extension�� $) └── var as str: bad.extension\xdca7\xdcfd command=`"/root/lsd/target/debug/lsd" "/tmp/.tmpRDHlAb"` code=0 stdout=`bad.extension\xdca7\xdcfd ` stderr=` ',

I was still trying to find a way to create the invalid name by shell, but no luck, python is my last choice...

codecov-io · 2020-10-07T09:07:17Z

Codecov Report

Merging #425 into master will increase coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   78.36%   78.53%   +0.16%     
==========================================
  Files          34       34              
  Lines        3342     3359      +17     
==========================================
+ Hits         2619     2638      +19     
+ Misses        723      721       -2

Impacted Files	Coverage Δ
src/meta/name.rs	`93.80% <100.00%> (+0.76%)`	⬆️
tests/integration.rs	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae92912...ef849b6. Read the comment docs.

0jdxt · 2020-10-07T09:59:24Z

tests/integration.rs

+    Command::new("python3")
+        .arg("-c")
+        .arg(format!(
+            r#"import pathlib; pathlib.Path('{}/bad.extension\udca7\udcfd').touch()"#,
+            tmp.path().to_str().unwrap()
+        ))
+        .output()
+        .expect("failed to create file");


try with double slashes, works for me

Suggested change

Command::new("python3")

.arg("-c")

.arg(format!(

r#"import pathlib; pathlib.Path('{}/bad.extension\udca7\udcfd').touch()"#,

tmp.path().to_str().unwrap()

))

.output()

.expect("failed to create file");

Command::new("sh")

.arg("-c")

.arg("touch $(printf 'bad.extension\\xdca7\\xdcfd')")

.output()

.unwrap();

have your try to run the code?

double backslashes fix the rust error report, but created a file named 'bad.extension\xdca7\xdcfd' not the utf 8 code.

I have done several tries before using python

Yeah I did a project with main.rs:

use std::process::Command; fn main() { Command::new("sh") .arg("-c") .arg("touch $(printf 'bad.extension\\xdca7\\xdcfd')") .output() .unwrap(); }

and when i run ls i get 'bad.extension'$'\334''a7'$'\334''fd'.

that's really weird, I can't reproduce your result

@zwpaper Are you running from within a linux docker image on a mac?

Is it possible that that could affect the result?

that's also my concern, so I was setting up a VM in fusion, big sur broke my VirtualBox

the same result on debian 10 in fusion vm.

@0jdxt could you help me to reproduce this?

I'll submit a PR just to see if it works with the CI.

zwpaper · 2020-10-08T13:16:22Z

@0jdxt thanks for the improvement

@meain This should be ready to go

0jdxt · 2020-10-08T13:26:35Z

I submitted a small pr to tidy my code a bit

zwpaper · 2020-10-17T01:56:26Z

@meain this is ready to go, what's your idea?

meain · 2020-10-17T04:25:44Z

Hey, I don't think there is a need to explicitly mention this in the readme under description as this is a really small edge case and an implementation detail.

zwpaper · 2020-10-17T08:48:29Z

@meain I added because this affects the user experience.

Users will get unexpected � when an invalid utf-8 appeared in the file name, so we should tell them about this.

meain · 2020-10-17T11:03:43Z

Yes, but the chances of user having an invalid utf-8 char in the filename is very unlikely.
Considering that, having this message in the project description does not seem right.

Plus, � seems like something that the user would be able to figure out to be a "non-standard" entry.
I am expecting only a few handful of users to ever run into this and if it ever comes to that, I think we could probably add it to the FAQ section of the readme.

fix lsd-rs#387

zwpaper · 2020-10-17T13:22:51Z

@meain moved to FAQ

0jdxt · 2020-10-17T13:26:37Z

hey, you somehow got rid of some of my commits? the tests now have the old macro. Did you do a rebase?

zwpaper · 2020-10-17T13:42:17Z

@0jdxt oh, it was my bad! I forgot to pull your work, so lucky that you found it!

0jdxt · 2020-10-17T13:44:18Z

Btw don't merge my pr its wrong atm

tidy up macro reduce reallocations macro to function tidy w/ import add EOL

zwpaper · 2020-10-17T14:43:42Z

@meain sorry for the accident, this should be ready to go

meain · 2020-10-18T05:21:19Z

Thanks @zwpaper and @0jdxt . Also thanks for filling in the CHANGELOG.md file.

zwpaper requested review from meain and Peltoche as code owners October 2, 2020 08:42

zwpaper force-pushed the master branch from 9179076 to 2363960 Compare October 4, 2020 14:54

0jdxt suggested changes Oct 6, 2020

View reviewed changes

zwpaper force-pushed the master branch from c7ed8f5 to 6f0a084 Compare October 7, 2020 09:05

zwpaper force-pushed the master branch 2 times, most recently from d37b5bf to 3529edc Compare October 7, 2020 09:46

0jdxt reviewed Oct 7, 2020

View reviewed changes

zwpaper and others added 6 commits October 17, 2020 20:54

skip extension if no ext found

c9aae19

fix lsd-rs#387

tidy up matches into maps

08dd4e8

test: 🔍 use python to create a bad extension

5b62d8f

fix test

2cf90f3

delete python dep

6ea2c1a

move how to show invalid utf-8 to FAQ

4a60549

zwpaper force-pushed the master branch from 34a1ea7 to 4a60549 Compare October 17, 2020 12:59

zwpaper and others added 2 commits October 17, 2020 22:04

add version sort, group name in gid, panic by invalid utf-8

23bb4a2

change macro to function in test

ef849b6

tidy up macro reduce reallocations macro to function tidy w/ import add EOL

zwpaper force-pushed the master branch from 8c7f810 to ef849b6 Compare October 17, 2020 14:20

meain merged commit 7c971e7 into lsd-rs:master Oct 18, 2020

meain mentioned this pull request Jan 14, 2021

Can't handle invalid encoding #9

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

skip extension if no ext found #425

skip extension if no ext found #425

zwpaper commented Oct 2, 2020

codecov-commenter commented Oct 2, 2020 •

edited

Loading

zwpaper commented Oct 2, 2020

Peltoche commented Oct 2, 2020

zwpaper commented Oct 2, 2020

0jdxt Oct 6, 2020 •

edited

Loading

zwpaper Oct 7, 2020 •

edited

Loading

zwpaper Oct 7, 2020

codecov-io commented Oct 7, 2020 •

edited

Loading

0jdxt Oct 7, 2020 •

edited

Loading

zwpaper Oct 7, 2020

0jdxt Oct 7, 2020 •

edited

Loading

zwpaper Oct 8, 2020

meain Oct 8, 2020

zwpaper Oct 8, 2020

zwpaper Oct 8, 2020

0jdxt Oct 8, 2020

zwpaper commented Oct 8, 2020

0jdxt commented Oct 8, 2020 •

edited

Loading

zwpaper commented Oct 17, 2020

meain commented Oct 17, 2020

zwpaper commented Oct 17, 2020

meain commented Oct 17, 2020

zwpaper commented Oct 17, 2020

0jdxt commented Oct 17, 2020

zwpaper commented Oct 17, 2020

0jdxt commented Oct 17, 2020 •

edited

Loading

zwpaper commented Oct 17, 2020

meain commented Oct 18, 2020

skip extension if no ext found #425

skip extension if no ext found #425

Conversation

zwpaper commented Oct 2, 2020

codecov-commenter commented Oct 2, 2020 • edited Loading

Codecov Report

zwpaper commented Oct 2, 2020

Peltoche commented Oct 2, 2020

zwpaper commented Oct 2, 2020

0jdxt Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

zwpaper Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

zwpaper Oct 7, 2020

Choose a reason for hiding this comment

codecov-io commented Oct 7, 2020 • edited Loading

Codecov Report

0jdxt Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

zwpaper Oct 7, 2020

Choose a reason for hiding this comment

0jdxt Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

zwpaper Oct 8, 2020

Choose a reason for hiding this comment

meain Oct 8, 2020

Choose a reason for hiding this comment

zwpaper Oct 8, 2020

Choose a reason for hiding this comment

zwpaper Oct 8, 2020

Choose a reason for hiding this comment

0jdxt Oct 8, 2020

Choose a reason for hiding this comment

zwpaper commented Oct 8, 2020

0jdxt commented Oct 8, 2020 • edited Loading

zwpaper commented Oct 17, 2020

meain commented Oct 17, 2020

zwpaper commented Oct 17, 2020

meain commented Oct 17, 2020

zwpaper commented Oct 17, 2020

0jdxt commented Oct 17, 2020

zwpaper commented Oct 17, 2020

0jdxt commented Oct 17, 2020 • edited Loading

zwpaper commented Oct 17, 2020

meain commented Oct 18, 2020

codecov-commenter commented Oct 2, 2020 •

edited

Loading

0jdxt Oct 6, 2020 •

edited

Loading

zwpaper Oct 7, 2020 •

edited

Loading

codecov-io commented Oct 7, 2020 •

edited

Loading

0jdxt Oct 7, 2020 •

edited

Loading

0jdxt Oct 7, 2020 •

edited

Loading

0jdxt commented Oct 8, 2020 •

edited

Loading

0jdxt commented Oct 17, 2020 •

edited

Loading