Skip to content

feat: Adding SingleFile appender#135

Merged
tisonkun merged 3 commits into
fast:mainfrom
nine9ths:single-file
Jun 2, 2025
Merged

feat: Adding SingleFile appender#135
tisonkun merged 3 commits into
fast:mainfrom
nine9ths:single-file

Conversation

@nine9ths
Copy link
Copy Markdown
Contributor

@nine9ths nine9ths commented May 31, 2025

I'm trying to switch to logforth from log4rs and everything is looking good except for the ability to just write to a single file. My application is a short running commandline utility that can write its log to either stderr or a file. I tried using the RollingFile appender with a Rotation::Never but I have a requirement that the log file path must be able to be set explicitly and RollingFile always includes the cnt. I've gated this behind a single-file feature similar to rolling-file.

This seemed like something that could be generally useful, hence the pull request.

Thanks for taking a look!

@nine9ths nine9ths changed the title Adding SingleFile appender feature(append/single_file): Adding SingleFile appender May 31, 2025
@tisonkun tisonkun changed the title feature(append/single_file): Adding SingleFile appender feat: Adding SingleFile appender May 31, 2025
Copy link
Copy Markdown
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @nine9ths!

Generally looks good. I'll take another look later and try to merge it. Feel free to ping me if I miss it by the end of next week.

Comment thread src/append/single_file/mod.rs Outdated
//! use logforth::append::single_file::SingleFileBuilder;
//! use logforth::layout::JsonLayout;
//!
//! let (file_writer, _guard) = SingleFileBuilder::new("/path/to/flile.log")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doc test would fail because /path/to/file.log typically no permission to access and non-exist.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, I always forget to make sure I've run the doc tests because I use nextest. Think it's passing now, ran with cargo test --doc --features single-file,json.

Copy link
Copy Markdown
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Merging ...

@tisonkun tisonkun merged commit deefb02 into fast:main Jun 2, 2025
9 checks passed
@tisonkun
Copy link
Copy Markdown
Contributor

I reviewed this patch today and noticed that if the RollingFile appender can set the full file name and doesn't always include cnt, you can use it with Rotation::Never to implement the necessity.

Related to #143.

@nine9ths
Copy link
Copy Markdown
Contributor Author

nine9ths commented Aug 29, 2025

Yes, I originally tried fulfilling my use case with Rotation::Never. I chose to implement a separate struct because the RollingFile appender was already feature gated so it seemed like the style of the library to put appenders behind gates and the single file appender would be a much simpler implementation.

I'm fine with switching over to RollingFile with Never if I can control the filename. I liked the suggestion of a pluggable file namer too.

@tisonkun
Copy link
Copy Markdown
Contributor

tisonkun commented Sep 7, 2025

Implementing at #159

@nine9ths welcome to give a review

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