Pwm1234/rotate on open

[pwm1234]

add argument to rotating file sink for rotate_on_open

This change adds logic to the rotating_file_sink constructor so the caller can control whether or not the log file is rotated at initialization. This change provides new functionality when the optional argument rotate_on_open is true the log file will be rotated first, so the current log begins in a new file. An old log file will be lost only if the maximum number has already been reached.

This is a refresh of PR #823.

[pwm1234]

Is this pull request acceptable to be merged into the main v1.x branch?

[gabime]

There was a review comment that i think is improtant to fix

[pwm1234]

I am sorry, but I am not sure what review comment needs to be fixed. Can give me a link?

[gabime]

can be optimized. why open and then immediately close in rotate?

[pwm1234]

I tried to address your comment. The optimization did introduce a little more complexity, but I think the changes are reasonable. I also updated to the latest v1.x commit. Are there other changes you want in this PR?

[gabime]

The optimization did introduce a little more complexity,

I was thinking about something simpler (without modifying rotate_()):

   rotating_file_sink(filename_t base_filename, std::size_t max_size, std::size_t max_files, bool rotate_on_open=false)
        : base_filename_(std::move(base_filename))
        , max_size_(max_size)
        , max_files_(max_files)
    {
        if (rotate_on_open)
        {
            rotate_();
            current_size_ = 0;  
        }
        else
        {
           file_helper_.open(calc_filename(base_filename_, 0));
           current_size_ = file_helper_.size(); // expensive. called only once
        }
    }

[pwm1234]

But the modification to rotate() is required because file_helper_.reopen(true) is called in rotate without first opening the file_helper_; this throws an exception. I only understand two choices either: 1) leave out the optimization or 2) introduce the complexity so we can avoid the call to reopen when open has not yet been called.

[gabime]

Good point. Let's leave out the optimization then. Maybe later we'll find a way to optimize without complicating the rotate_() too much.

[pwm1234]

I agree that the minor optimization is not worth the complexity. I made a commit that removes the undesirable complexity with rotate_(). This final(?) change avoids fixes an error by not rotating an empty file. Please let me know if you have any other changes for this PR. If not, I look forward to having this PR be part of spdlog. Thank you for your help and for making spdlog available.

[gabime]

Merged. Thanks @pwm1234