Implement option for printing custom formats by tmccombs · Pull Request #1043 · sharkdp/fd (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits1 Checks18 Files changed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This is mostly a proof of concept before merging there is still some work that needs to be done for:
- Write new tests (both unit tests and integration tests)
- Add documentation to man page
- Potentially add support for outputting arbitrary bytes on unix like we do for normal printing (I'm not going to worry about this for now)
- Benchmark, and optimize if necessary
- one potential optimizaiton could be to write directly to the stream rather than building an intermediary OsString, I started looking into that, but it got kind of messy.
Also, this doesn't currently support colorized output when using format string. I think that for an initial implementation that is probably fine.
Thank you very much for taking a stab at this!
I didn't have time to properly review it, but I think I am in favor of integrating this new option! That is, unless it leads to a performance penalty for the default case (without --format).
Something that I thought about in a different context, but seems fitting here: I'm not a huge fan of the {}/{/}/{//}/{.}/… syntax, which we borrowed from GNU parallel, I believe. Would it make sense to think about something a bit more verbatim? I think we could easily support something like {path}/{basename}/{parent}/{without_extension}/… in addition. What do you think? I'd much rather type out those words than having to look up the syntax each time.
I like the more verbose names too.
👍. We can also do this in a follow up PR though.
Regardless of the short/long syntax discussion, I'd like to see this being integrated to fd!
@tmccombs Are you still interested in finishing this? It would probably be best to focus on the benchmarks first, to make sure that we don't run into performance regressions.
Sorry it took me so long to get back to this. I am interested in finishing this. Though I wouldn't mind some help with benchmarking it.
It would be great to pick this up again. Anything I can do? I can definitely take care of the benchmarks. I could also take a look at fixing the merge conflicts (?).
I fixed the merge conflicts.
One question I have is if we would rather have a --printf option with more format optiions, such as access and modified times, owning user and group, etc. Certainly not as extensive as find, but more than just variants of the path. Although, I really don't love the idea of having two different format syntaxes.
One question I have is if we would rather have a --printf option with more format optiions, such as access and modified times, owning user and group, etc. Certainly not as extensive as find, but more than just variants of the path. Although, I really don't love the idea of having two different format syntaxes.
I would personally like if we extend the {…} syntax. For example, we could have more descriptive aliases like {basename} instead of {/}. This would allow us to use something like {access_time} and similar if we want to add more fields, without having to come up with a cryptic/concise syntax. If we need more control over how a field like a date is printed, we could follow Python f-strings syntax with something like {access_time:%Y-%m-%d}. I don't think that we need to have a find -printf compatibility mode here. What do you think?
@tmccombs @sharkdp - Any chance this will be merged soon?
I'm dying for this feature and compulsively checking this PR every few days as a result. Please put me out of my misery lol.
| long, |
|---|
| value_name = "fmt", |
| help = "Print results according to template", |
| conflicts_with = "list_details" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much and sorry for the late review!
I've run a few benchmarks with hyperfine, and haven't noticed a significant difference in performance.