Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --custom-ignorefile command line flag #74

Merged
merged 6 commits into from
Dec 11, 2023

Conversation

ProspectPyxis
Copy link
Contributor

This adds the --ignore-file command line option to treat each listed file like a .gitignore. This is distinct from the --exclude-file option, since it uses the walk builder and thus will match the filename even in subdirectories.

The name of the flag could be changed - as it stands, I don't think it's very clear from a glance what the difference between this flag and --exclude-file is - but this name seems like a good enough choice for now.

Closes rustic-rs/rustic#832.

@simonsan simonsan added C-enhancement Category: New feature or request A-commands Area: Related to commands in `rustic_core` labels Nov 29, 2023
@aawsome
Copy link
Member

aawsome commented Nov 29, 2023

Thanks @ProspectPyxis for the PR!
I think this solves rustic-rs/rustic#832 directly once we publish a new rustic_core version and bumb the rustic dependency.

I have one thing: Is the name of the option a good choice? I personally think that --ignore-filename could be also a better fit, but I am open to discuss it...

Update: I agree to @simonsan that --ignore-filename is also not a good idea...

@ProspectPyxis
Copy link
Contributor Author

Yes, I agree - the name feels a little awkward as it stands. There's not exactly a good short name that encapsulates "treat this file as a gitignore, so this works even in subfolders"...

I can make a quick commit to fix this if you believe that would be a better name.

@simonsan
Copy link
Contributor

IMHO ignore-{filename, file} are both bad from a UX perspective, as they sound more like if this file or this filename should be ignored, rather than a path to a file that contains filenames that should be ignored.

@aawsome
Copy link
Member

aawsome commented Nov 29, 2023

@simonsan do you have a better idea?

@simonsan
Copy link
Contributor

Some things that are IMHO better fitting:

  1. --ignore-list <filename>
  2. --exclude-from <filename>
  3. --ignore-file-list <filename>
  4. --skip-list <filename>
  5. --ignore-file-input <filename>
  6. --ignore-file-source <filename>
  7. --exclude-file-list <filename>
  8. --file-ignore-source <filename>
  9. --ignore-source <filename>

@ProspectPyxis
Copy link
Contributor Author

ProspectPyxis commented Nov 29, 2023

I like --ignore-source, personally. I don't think we can fully avoid ambiguity here, but that's pretty understandable to me as "use this filename as a source just like gitignore". I could also update the documentation to refer to a filename rather than a file, just to be even more clear.

@simonsan
Copy link
Contributor

simonsan commented Nov 29, 2023

I tend to like all the one's containing the word list better, as they make clear, that this is points to a list for ignore-related things. We should think about it like what the path that this option points to contains, and this contains a Vector of things that we abstract away as a file. Hence, I would go with something that contains the word list.

ignore-source has IMHO the same problem as the one's before, as it make clearer that this could be a source for ignore (something), but doesn't make clear, that this would be a list of things. But it's better than the initial one's.

P.S.: I generated the list with ChatGPT, so I'm not preferring my own ideas here, rather talking about it from a 3rd person POV.

@aawsome
Copy link
Member

aawsome commented Nov 29, 2023

I'd like to throw in --use-ignorefile or --custom-ignorefile to make clear that we are talking about ignorefiles and not files to directly ignore.
I'll keep thinking about what would be a good and least misleading option name ;-)

@simonsan
Copy link
Contributor

I'd like to throw in --use-ignorefile or --custom-ignorefile to make clear that we are talking about ignorefiles and not files to directly ignore. I'll keep thinking about what would be a good and least misleading option name ;-)

Or --ignore-from-source for example. But yes, other options much appreciated! :)

@aawsome
Copy link
Member

aawsome commented Dec 4, 2023

Actually I think we should have the term ignorefile in the option - anyone can google that term and gets an impression about what we are speaking here. Everything that has just ignore or file IMO is ambiguous as it can indicate we specify something to directly ignore or that we specify a file directly (which we don't as we don't give a path to a file). Also filename doesn't seem a good choice - users may think this is similar to file...

I don't like the *list* options as in the CLI, it will appear as option that can be multiple specified, but something like --ignorefile-list .rustic-ignore IMO is counter-intuitive.
I also don't like the *source*, *input*, *from* as users could think we specify a source or input where we directly get something to ignore, but we don't - we specify a name of ignorefile(s).

So, I'd personally prefer --custom-ignorefile. What is your opinion?

@simonsan
Copy link
Contributor

simonsan commented Dec 4, 2023

I don't like the *list* options as in the CLI, it will appear as option that can be multiple specified, but something like --ignorefile-list .rustic-ignore IMO is counter-intuitive.

Oh, I actually thought it should be? Like it should end up in a Vec<Path> so we can specify multiple ignore files to read stuff from? It can be in most cases only one, but maybe you want to specify multiple, like a general one, and then one for a certain snapshot command or so? So for that you would add another --<specify-ignore-file-command> <profile-dependent-ignore-file-path>.

@aawsome
Copy link
Member

aawsome commented Dec 10, 2023

So for that you would add another --<specify-ignore-file-command> <profile-dependent-ignore-file-path>.

Yes, this is how clap works in that case.

As a didn't hear arguments against --custom-ignorefile, let's just use this and complete this PR!

@ProspectPyxis ProspectPyxis changed the title feat: add --ignore-file command line flag feat: add --custom-ignorefile command line flag Dec 11, 2023
@ProspectPyxis
Copy link
Contributor Author

Updated as requested!

@aawsome aawsome enabled auto-merge December 11, 2023 08:23
@aawsome aawsome added this pull request to the merge queue Dec 11, 2023
Merged via the queue into rustic-rs:main with commit f354a0c Dec 11, 2023
46 checks passed
@simonsan
Copy link
Contributor

Updated as requested!

Thank you for the PR! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-commands Area: Related to commands in `rustic_core` C-enhancement Category: New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a different file name other than .gitignore for file-based ignoring
3 participants