Problem due to overloading `no-confirm` for "save" about denote

[mentalisttraceur]

Thinking about it a bit more, I think pulling save into the lowest-level-but-still-public-API rewrite function is probably not the right play - better to just empower composition.

So for point 2 above, I now recommend the return value approach - just allowing the caller to know after the call if the rewrite happened is more flexible than pulling the save into the rewrite.

[jeanphilippegg]

You are right. The coupling is not ideal.

In my fork of the code (not announced yet elsewhere), I have made the options denote-rename-confirmations and denote-save-buffers control how the renaming commands confirm and save buffers. All renaming commands will respect those variables. None will have any optional parameters anymore regarding that.

denote-rename-confirmations will default to '(add-front-matter rewrite-front-matter modify-file-name) and denote-save-buffers to nil.

In the release notes for version 2.3.0, Prot has expressed a wish to wait until late April for new features.

I plan on making the pull requests to include everything that can be included back from my fork into Denote.

But first, there will be 2 other pull requests that will come before this one: I will first add denote-forced-* variables to help in customizing denote commands and then I will submit a refactor of all renaming commands to remove all code duplication and enhance their flexibility. It will be very easy for developers as well as users to define their own custom renaming commands.

It will be very easy to have separate renaming commands for all file name components (or multiple).

This is all done already in my fork. I will make it public this weekend, but I will do the pull requests in May!

[protesilaos]

Yeah, the idea is to wait a bit for new features or more fundamental changes so that we can fix obvious bugs. Once we are sure that users are okay with what they have, we resume the work. A more fine-grained control over these confirmations is a good idea.

[mentalisttraceur]

Yep, I saw that in the release notes and totally respect that - I'm raising this now because this could be seen as a bug, and point 1 of my proposal as the fix.

Consider: if I hadn't read the changelog and checked the docstrings of functions I use, I wouldn't have realized that code which I wrote+tested+trusted to only save when deemed safe by my checks would now suddenly start saving files in other situations as well.

Personally, I don't mind - I already protected myself from this by proactively thoroughly checking the change log* and immediately adjusting my code. Cool, no data loss happened, no bad feelings.

But I think users would have been justified in expecting that the meaning/effect of a parameter named "no confirm" in a public API function wouldn't get promoted to "no confirm and save".

On the other hand, if you've got one or two users who prefer the new behavior, maybe that's more representative of a typical user. And the odds of serious data loss from this are probably low? So I'm not saying that you definitely should treat this as a bug, I'm just suggesting to think about it.

* Speaking of, thank you very much for that thorough change log! I know those are really hard to write even in basic form, and yours is both thorough and has an impressive degree of polish.

[protesilaos]

I just merged the contribution of @jeanphilippegg which should fix this issue. Please check.

[mentalisttraceur]

@protesilaos thanks! I just read over most of that change, and it looks good to me.

[protesilaos]

Thank you!