RFC range-diff: treat notes like `log`
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest3 patches 4 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | range-diff: treat notes like `log`range-diff.c revision.c t/ | +34 | 2 replies | |
| 1/3 | range-diff: treat notes like `log`range-diff.c revision.c t/ | +34 | 0 replies | |
| 2/3 | doc: pretty-options: remove documentation for deprecated optionsDocumentation/ | +0 | 0 replies | |
| 2/3 | doc: pretty-options: remove documentation for deprecated optionsDocumentation/ | +0 | 0 replies | |
| 3/3 | revision: comment `--no-standard-notes` as deprecatedrevision.c | +1 | 0 replies | |
| 3/3 | revision: comment `--no-standard-notes` as deprecatedrevision.c | +1 | 2 replies |
Reviews trailers per patch
None of the 6 patches has a Reviewed-by or Acked-by.
| Patch | Johannes Schindelin |
|---|---|
| Series | Co-authored-by Co-authored-by Co-authored-by Co-authored-by |
| 1/3 | |
| 1/3 | |
| 2/3 | |
| 2/3 | |
| 3/3 | |
| 3/3 |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [RFC PATCH 0/3] range-diff: treat notes like `log`
Currently, range-diff shows the default notes if no notes-related arguments are given. This is also how log behaves. But unlike range-diff, log does *not* show the default notes if --notes=<custom> are given.
These changes are supposed to make format-range behave like log with regards to notes.
These changes also fixes an issue with notes being shared in the cover letter via range-diff, and that’s really the main motivation for making these changes.
§ How log works
log shows the default notes if no notes arguments are given. But if you give it specific notes to show then it forgets about the default notes. Further, there is the convenience --notes option which will show the default notes again. These options are cumulative. For example:
git log --notes --notes=custom
Will show the default notes as well as the custom notes.
See discussion in: https://lore.kernel.org/git/[email protected]/
§ How range-format works
range-format passes --notes to log, which means that it does not have the default behavior of log (forget the default logs if you say e.g. --notes=custom). However, the man page says that (under --[no-]notes[=<ref>]):
Quoted 1 line
This flag is passed to the git log program (see git-log(1)) that generates the patches.
This makes me (at least) think that range-format is supposed to work just like log with regards to notes.
§ format-patch and the difference between showing and sharing
format-patch has a different default: it shows no notes. This makes sense in my opinion since format-patch is meant to be used to share changes with others, and you might be surprised if your notes (which might have only been notes to yourself) are sent out in your emails (keep in mind that notes refs are *not* pushed by default).
But the slightly faulty behavior of range-diff bleeds through to format-patch since the latter calls the former; if you have default notes they can be shared in the range-diff on the cover letter, even though format-patch isn’t supposed to show them.
§ Code layout and approach to the problem
As I’ve said, my focus was on fixing format-patch, so I’ve considered how format-patch calls range-diff which in turn calls log.
format-patch is a command which is defined in builtin/log.c. For notes in particular it in fact has some explicit logic for handling notes based on the value of rev. (There seems to be no issues with this part of the code; only the code in range-diff.c which passes the default argument to log.) It then calls range-diff.c:show_range_diff. That function on master loads some default arguments, among them --notes. It then eventually calls log as a subprocess.
My change consists of co-opting the deprecated --standard-notes and changing its behavior so that it can be used in range-diff.c:show_range_diff.
Using a special switch/option was the only way I found in order to fix this problem.
I could have also created a new option but I thought that doing a smaller change initially would be better.
§ RFC
This is marked as RFC since I chose to co-opt a deprecated option in order to get range-format to work, in my opinion, as it should.
§ Carbon copy
Based on contrib/contacts/git-contacts master..@.
Kristoffer Haugsbakk (3):
range-diff: treat notes like log
doc: pretty-options: remove documentation for deprecated options
revision: comment --no-standard-notes as deprecated
Documentation/pretty-options.txt | 1 - range-diff.c | 2 +- revision.c | 8 ++++++-- t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-)
Signature · 1 line
2.41.0.rc2