ls/rm/cd strip ansi escapes

[panicbit]

Describe the bug

ls, rm, cd and possibly other tools strip ansi escapes, rendering some file operations impossible or resulting in surprising and dangerous behaviour.

How to reproduce

1. touch $"(ansi red)hello"
2. rm touch $"(ansi red)hello"
3. observe rm failing to find the file

Expected behavior

I expect core filesystem commands to do as I say.

Screenshots

Configuration

key	value
version	0.66.4
branch
commit_hash	8783742
build_os	linux-x86_64
build_target	x86_64-unknown-linux-gnu
rust_version	rustc 1.62.1 (e092d0b6b 2022-07-16)
rust_channel	stable-x86_64-unknown-linux-gnu
cargo_version	cargo 1.62.1 (a748cf5a3 2022-06-08)
pkg_version	0.66.4
build_time	2022-08-13 17:05:47 +02:00
build_rust_channel	release
features	database, dataframe, default, trash, which, zip
installed_plugins

Additional context

No response

[fdncred]

seems like an edge case where people would actually name files using ansi escapes. we talked about this when we decided to strip them for file move operations. maybe there should be some string literal way to remove files when mistakes like this happen?

[panicbit]

What is the usecase for automatically stripping escapes from a path? It seems to me like that is more of an edgecase. Especially considering that there is already a mechanism to strip escapes explicitly if you really need to via ansi strip.
There will always be one or the other command that won't do the stripping and then someone will be surprised by it (in fact, touch and mkdir don't strip right now, already causing an inconsistent behaviour).
This stripping is simply causing much more issues than it solves IMO.

[panicbit]

Ok, I see what the original reason in #6220 was regarding find.
IMO this is the wrong approach to solve it and more of an issue with how find works.

=> ls can colorize table rendering without modifying the content, why can't find?

[panicbit]

Maybe we could extend the PipelineMetadata a bit for the find usecase?

nushell/crates/nu-protocol/src/pipeline_data.rs

Lines 51 to 59 in 72c27bd

	#[derive(Debug, Clone)]
	pub struct PipelineMetadata {
	pub data_source: DataSource,
	}
	#[derive(Debug, Clone)]
	pub enum DataSource {
	Ls,
	}

While I'd love to see a more generic approach to modify the rendering of the table without having to hardcode specific commands, adding find as another special case should work for now.

[kubouch]

We could strip ansi sequences for touch and other "sink" commands that do not pass data further. But the metadata approach could be a good fit as well.

[kubouch]

The use case for stripping ansi sequences is if you want to do something like ls | find blah | each {|it| rm $it.name }

[fdncred]

I'm always open to a PR with a better solution.

[panicbit]

Already working on it!

[panicbit]

Found a bug in find while digging through its code X)

I'll fix it along the way. We should be able to cherry-pick the fix if we don't end up accepting my full changeset.

[panicbit]

Oh, looks like it's even worse; there's another bug that is independent from use_ls_colors:

(I'm so sorry 😅)

[fdncred]

No worries. I'm glad you're finding and fixing these. It was a real pain to make find highlight things.

[panicbit]

I'm not gonna ask about what's happening here 😅 :

[panicbit]

#6324 should lay the groundwork for fixing this issue.

[sholderbach]

Boosting this one as something that we should probably resolve in a principled way before 1.0 .

The current behavior of manipulating the data to deal with how find or other commands work feels deeply unserious.

Nushell should allow you to compose data pipelines reliably. And if we partially made that work for ls that you can get highlighting that doesn't affect things downstream, we should have the same experience with find if possible. Undocumented stripping seems to cause more potential harm.

See also #6220 (comment)