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

New collector DirectoryEntries #11

Merged
merged 6 commits into from
Mar 4, 2022
Merged

Conversation

Enselic
Copy link
Contributor

@Enselic Enselic commented Feb 28, 2022

The use case I have in mind is to add info about custom assets to bat. If we add this line to bat:

        .info(DirectoryEntries::new("Custom assets", PROJECT_DIRS.cache_dir()))

we will get output in --diagnostics that look like this if custom assets are installed:

#### Custom assets

- metadata.yaml, 101 bytes
- syntaxes.bin, 726517 bytes
- themes.bin, 40475 bytes

and like this

#### Custom assets

'/Users/martin/.cache/bat' not found

or this

#### Custom assets

'/Users/martin/.cache/bat' is empty

if no custom assets are installed.

I took some liberties in the structure and interface of the new collector.

  • I put it in its own file to make it easier to work with
  • I made the title owned rather than referenced so that e.g. a collector can be constructed in a helper function with the title from a local var in the function. See new_with_title_from_local_variable() example in tests. That code does not work for e.g. FileContents.
  • I used the "impl trait" syntax for clarity

I still made the new collector appear in the right place in the module hierarchy though, as can be demonstrated with cargo public-items:

% cargo public-items | grep -e DirectoryEntries -e FileContent
 Documenting bugreport v0.4.1 (/Users/martin/src/bugreport)
    Finished dev [unoptimized + debuginfo] target(s) in 0.28s
pub fn bugreport::collector::DirectoryEntries::collect(&mut self, _: &CrateInfo<'_>) -> result::Result<ReportEntry, CollectionError>
pub fn bugreport::collector::DirectoryEntries::description(&self) -> &str
pub fn bugreport::collector::DirectoryEntries::new(title: impl Into<String>, path: impl Into<PathBuf>) -> Self
pub fn bugreport::collector::FileContent::collect(&mut self, _: &CrateInfo<'_>) -> result::Result<ReportEntry, CollectionError>
pub fn bugreport::collector::FileContent::description(&self) -> &str
pub fn bugreport::collector::FileContent::new<P: AsRef<Path>>(title: &'a str, path: P) -> Self
pub struct bugreport::collector::DirectoryEntries
pub struct bugreport::collector::FileContent<'a>

or just a quick look at the cargo doc HTML if you're old-school :)

This is pretty bare-bones in terms of functionality (see docs in the code for limitations and idea for future improvements) but it should suit the bat use case well.

@Enselic
Copy link
Contributor Author

Enselic commented Mar 1, 2022

It would also be valuable to use FileContent on metadata.yaml in bat to get additional details about custom assets, but I would say there is some utility in this new collector too.

Btw, seems like CI has hanged, but that seems to be fixed by #12.

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much. This looks great!

src/collector/directory_entries.rs Outdated Show resolved Hide resolved
src/collector/directory_entries.rs Outdated Show resolved Hide resolved
tests/test_collector_directory_entries.rs Show resolved Hide resolved
@sharkdp sharkdp merged commit 9579f4e into sharkdp:master Mar 4, 2022
@sharkdp
Copy link
Owner

sharkdp commented Mar 4, 2022

Thank you. Let me know when/if I should release a new version in order to integrate this into bat.

@Enselic Enselic deleted the directory-entries branch March 29, 2022 07:20
@Enselic
Copy link
Contributor Author

Enselic commented Mar 29, 2022

@sharkdp If you could make a release of bugreport when you get time that would be great. (I wanted to wait a bit to see if I could think of something else to do before a release but nothing has come to mind.)

@sharkdp
Copy link
Owner

sharkdp commented Apr 1, 2022

Yes, of course: https://crates.io/crates/bugreport/0.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants