| Message ID | 9141f5a86e66276b672fc54783afe3b48b6227cf.1686505920.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | range-diff: treat notes like `log` | expand |
Kristoffer Haugsbakk <code@khaugsbakk.name> writes: > We still use `--standard-notes` but this option has no use and is no > longer documented anywhere. > > Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> > --- > revision.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/revision.c b/revision.c > index a0ab7fb784..24219c741a 100644 > --- a/revision.c > +++ b/revision.c > @@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg > enable_default_display_notes(&revs->notes_opt, > &revs->show_notes); > revs->notes_opt.use_default_notes = -1; > + /* Deprecated */ > } else if (!strcmp(arg, "--no-standard-notes")) { > revs->notes_opt.use_default_notes = 0; > } else if (!strcmp(arg, "--oneline")) { With the placement of this new comment, it is unclear which one between "--standard-notes" and "--no-standard-notes" is getting deprecated (actually, the comment is placed inside the block for the former, so it may be more natural to interpret that the comment marks the former as deprecated). } else if (!strcmp(arg, "--no-standard-notes")) { + /* Deprecated */ revs->notes_opt.use_default_notes = 0; } I am not commenting if it makes sense to declare that the option is deprecated here---I'll leave it to others to argue for/against it. The usual reasoning to add/maintain "--no-foo" is so that we can get "cmd --foo --no-foo" to naturally work, but it does not apply here?
On Tue, Jun 13, 2023, at 00:21, Junio C Hamano wrote: >> [snip] >> diff --git a/revision.c b/revision.c >> index a0ab7fb784..24219c741a 100644 >> --- a/revision.c >> +++ b/revision.c >> @@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg >> enable_default_display_notes(&revs->notes_opt, >> &revs->show_notes); >> revs->notes_opt.use_default_notes = -1; >> + /* Deprecated */ >> } else if (!strcmp(arg, "--no-standard-notes")) { >> revs->notes_opt.use_default_notes = 0; >> } else if (!strcmp(arg, "--oneline")) { > > With the placement of this new comment, it is unclear which one > between "--standard-notes" and "--no-standard-notes" is getting > deprecated (actually, the comment is placed inside the block for the > former, so it may be more natural to interpret that the comment > marks the former as deprecated). > > } else if (!strcmp(arg, "--no-standard-notes")) { > + /* Deprecated */ > revs->notes_opt.use_default_notes = 0; > } Thanks, I’ll use that in the next version if this patch is still included. > I am not commenting if it makes sense to declare that the option is > deprecated here---I'll leave it to others to argue for/against it. > The usual reasoning to add/maintain "--no-foo" is so that we can get > "cmd --foo --no-foo" to naturally work, but it does not apply here? Yeah, because I decided to piggyback on a deprecated command in order to make the change. So this change moves `--standard-notes` from “deprecated” to “for internal use”, while `--no-standard-notes`’s status remains the same, which is “deprecated”. Unless the symmetry of `--[no-]standard-notes` turns out to be useful. Like I’ve said before, I would have preferred to find a way to solve this issue without using an internal switch. But I wasn’t able to. Cheers
diff --git a/revision.c b/revision.c index a0ab7fb784..24219c741a 100644 --- a/revision.c +++ b/revision.c @@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg enable_default_display_notes(&revs->notes_opt, &revs->show_notes); revs->notes_opt.use_default_notes = -1; + /* Deprecated */ } else if (!strcmp(arg, "--no-standard-notes")) { revs->notes_opt.use_default_notes = 0; } else if (!strcmp(arg, "--oneline")) {
We still use `--standard-notes` but this option has no use and is no longer documented anywhere. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> --- revision.c | 1 + 1 file changed, 1 insertion(+)