search

eza-community / eza

A modern, maintained replacement for ls
https://eza.rocks
MIT License
8.79k stars 170 forks source link

feat(filter): --cachedir-ignore option #949

Open SIGSTACKFAULT opened 2 months ago

SIGSTACKFAULT commented 2 months ago

Description

Fixes #948.

adds the --cachedir-ignore option which causes eza to ignore directors with a CACHEDIR.TAG and the correct magic number. see https://bford.info/cachedir/

How Has This Been Tested?

nix flake check passes, but I haven't written formal tests. that's why it's a WIP.

TODO

SIGSTACKFAULT commented 2 months ago

oop i thought adding [WIP] made it a draft automagically

SIGSTACKFAULT commented 2 months ago

I added a directory with a CACHEDIR.TAG to itest; should I update all the other tests or put the new directory somewhere else?

SIGSTACKFAULT commented 2 months ago

might change it from --cachedir-ignore to --ignore-cachedir because i was copying --git-ignore but it makes more sense as --ignore-cachedir

MartinFillon commented 2 months ago

Hey, first thing, thanks for the pr, I am going to take a deeper look at it wen I have time. Please make sure to write tests cases and rerun the necessary tools.

PThorpe92 commented 2 months ago

Awesome, thanks for the PR!

I like the idea for sure, I'll also check it out later on

SIGSTACKFAULT commented 2 months ago

Hey, first thing, thanks for the pr, I am going to take a deeper look at it wen I have time. Please make sure to write tests cases and rerun the necessary tools.

That was one of my questions -- to add a test case i would have to add stuff presumably to the itest directory, but doing that would break all the existing tests. should I update all the existing tests? create a new directory? some third thing?

SIGSTACKFAULT commented 2 months ago

As this is a feature that reads data could you make it a feature in cargo toml, make it a default, just to be sure it can be disabled by user not wanting any of that

as in, a feature that deletes the flag entirely, makes the flag a no-op, makes it not check the magic number, or what? it already only bothers to check for CACHEDIR.TAG if the flag is passed.

MartinFillon commented 2 months ago

as in, a feature that deletes the flag entirely, makes the flag a no-op, makes it not check the magic number, or what

yes

SIGSTACKFAULT commented 2 months ago

which one do you mean by "yes"? i don't understand

MartinFillon commented 2 months ago

All of them, as I already had previous users reaching out on adding features needing dependencies or having small systems, I think that having the option to disable file reading is not a bad idea, if you have thought lmk.

PThorpe92 commented 2 months ago

Hey finally had a chance to look this over. This looks good man. :+1: Just the one minor fn param.

Seeing as this needs the flag to be enabled, I dont see a reason why it needs to be behind a feature flag.

I do agree with cafk, if we could rebase some of those commits into a couple/few logical conventional commits. This is good to go for me :+1:

MartinFillon commented 2 months ago

Seeing as this needs the flag to be enabled, I dont see a reason why it needs to be behind a feature flag.

I will take a look at the code to be sure but I trust you as you read the code, didn't had time for the moment tho.

SIGSTACKFAULT commented 2 months ago

is that squashed enough or do you want more?

MartinFillon commented 2 months ago

Looks good enough

SIGSTACKFAULT commented 1 month ago

Yeah, i know it needs tests. i've been nerd-sniped by bevy, sorry >.<

SIGSTACKFAULT commented 1 month ago

just itest fails on main so i think i'm waiting for #959