Implement TarOperation and TarParam foundations#49
Conversation
clippy and cargo fmt
|
Just wanted to give this a bump, @kaladron or @sylvestre let me know if we need anything. |
|
Wanted to give this another bump @sylvestre @cakebaker. The PR after this I plan to rework the Clap arg processing so we can not only take care of @sylvestre suggestion to help make TarOperation more sane but also see if we can start addressing #55. |
|
Sorry, it's performance review season at work and a lot of things in life
are delayed at the moment.
…On Sun, Dec 14, 2025, 18:55 Nicholas Still ***@***.***> wrote:
*stillbeingnick* left a comment (uutils/tar#49)
<#49 (comment)>
Wanted to give this another bump @sylvestre <https://github.com/sylvestre>
@cakebaker <https://github.com/cakebaker>. The PR after this I plan to
rework the Clap arg processing so we can not only take care of @sylvestre
<https://github.com/sylvestre> suggestion to help make TarOperation more
sane but also see if we can start addressing #55
<#55>.
—
Reply to this email directly, view it on GitHub
<#49 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC4FO5SVFIW4Q42Y4RDUID4BWXAJAVCNFSM6AAAAACM7IZVVWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMNJRHAYTANZUHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
All good @sylvestre reviewed unless you are a maintainer now too @kaladron. But always open to more reviews. |
Removing for match loop going over all matched args. Moved to standard if get_* for specific matches.
|
@cakebaker Thank you for the review! Items requested in the review are taken care of. |
|
Every time I somehow forget to run clippy and fmt! I need CI for my brain. Should be good to go now. |
clippy and cargo fmt
Removing for match loop going over all matched args. Moved to standard if get_* for specific matches.
clippy and cargo fmt
Removing for match loop going over all matched args. Moved to standard if get_* for specific matches.
This reverts commit 4bef208.
… feat/taroperation-tarparam
Adding TarParam and TarOperation framework clippy and cargo fmt changing arg match taroption processing Removing for match loop going over all matched args. Moved to standard if get_* for specific matches. fmt and derive default tarparams
clippy and cargo fmt changing arg match taroption processing Removing for match loop going over all matched args. Moved to standard if get_* for specific matches. fmt and derive default tarparams cargo workspace dependency fixes Adding TarParam and TarOperation framework clippy and cargo fmt changing arg match taroption processing Removing for match loop going over all matched args. Moved to standard if get_* for specific matches. Revert "changing arg match taroption processing" This reverts commit 4bef208. changing arg match taroption processing Removing for match loop going over all matched args. Moved to standard if get_* for specific matches. cargo workspace dependency fixes Adding TarParam and TarOperation framework clippy and cargo fmt changing arg match taroption processing Removing for match loop going over all matched args. Moved to standard if get_* for specific matches. fmt and derive default tarparams
… feat/taroperation-tarparam
|
Sorry @sylvestre and @cakebaker for the absolute mess of commit history something blew up and my local squashed just fine but the PR commits and the branch in my fork won't squash. So I am not sure if it is github or most likely a me issue. |
| type Error = TarError; | ||
| fn try_from(value: &str) -> Result<Self, Self::Error> { | ||
| match value { | ||
| "concate" => Ok(Self::Concatenate), |
There was a problem hiding this comment.
| "concate" => Ok(Self::Concatenate), | |
| "concatenate" => Ok(Self::Concatenate), |
no ?
There was a problem hiding this comment.
| "concate" => Ok(Self::Concatenate), | |
| "catenate" | "concatenate" => Ok(Self::Concatenate), |
Ya actually both gnu tar takes both -> All Tar options, BSD tar doesn't have concatenate. Also correct the spelling in this one, thank you for the catch!
| /// [`TarParams`] Holds common information that is parsed from | ||
| /// command line arguments. That changes the current execution of | ||
| /// tar. | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
can we avoid this ?
it is usually a sign that the code needs to be refactored
There was a problem hiding this comment.
Turns out the lint wasn't needed. But there was dead enum variants and methods for TarParams and Options, those were removed and commited.
| } else { | ||
| // TODO: update messaging | ||
| Err(Box::new(TarError::TarOperationError( | ||
| "Error processing: Operations".to_string(), |
There was a problem hiding this comment.
yeah, we should probably provide more information in case of errors, no?
There was a problem hiding this comment.
Yes I added the ids of the arguments passed in the error message so if the error is ever called it will provide some information. Clap will catch the malformed or non-existent args so this error message should never get executed but if it does we will have a thread to pull on.
In the future once we get more consistent language we can change the operation error messaging.
|
linter is failing |
|
I keep thinking about this every month or so. The problem is that adding this up front, I have no way of knowing if this is a good approach or not, and then I worry about us having infrastructure that we feel we have to use instead of refactoring up to good infra that meets out needs. As such, I would actually prefer to close this and figure this out once we have a set of features that demand it. Sorry for the delay in coming to this conclusion. =( I should have been faster about it. |
|
@kaladron I get not wanting to have the over-head but we need to have a plan. A sense of direction for this. I took what was already laid out in GNU tar and attempted to create a modular solution so we could impl one piece at a time. I wanted to put something in just to get the discussion started. I created charts and documentation so we could hate on it and make it better. I don't want uutar to be mine I want it to be ours. If my design sucks lets come up with a better one. If we want tweaks lets tweak it. I don't want to build uutar in silos never discussing anything. This project needs input, discussion, comments. It doesn't have a design, it doesn't have a vision, (at least a published one) and it doesn't have a strategy. It desperately needs it. TAR is such a mission-critical piece of everyday life it deserves the full-send. Here is my first crack at a vision statement, problem statement, and principles:
These are incomplete but I really just want us to decide on a direction. Again I am really welcoming feedback and input on this. We need to get some guardrails and direction so we can foster the support and acquire hands to help carry the load. @sylvestre @cakebaker would also love you two to weigh in. |
4 participants
This PR builds on the great work done by @kaladron and lays out the foundation to enable adding operations and options moving forward. Leveraging rust enums and building behavioral control with the enum dispatch pattern.
There are 3 critical pieces to this foundational layer.
TarParams
TarOptions
TarOperation
While this is just the bones it will continue to morph and change as more functionality is built into tar.