* allow a differn't diff.context config for git format-patch @ 2019-04-14 21:48 Shawn Landden 2019-04-23 3:06 ` Jeff King 0 siblings, 1 reply; 7+ messages in thread From: Shawn Landden @ 2019-04-14 21:48 UTC (permalink / raw) To: git When I send patches I want them to have lots of context, but when just looking at a diff, I can always open the file for context. -SHawn ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: allow a differn't diff.context config for git format-patch 2019-04-14 21:48 allow a differn't diff.context config for git format-patch Shawn Landden @ 2019-04-23 3:06 ` Jeff King 2019-04-23 3:45 ` Junio C Hamano 0 siblings, 1 reply; 7+ messages in thread From: Jeff King @ 2019-04-23 3:06 UTC (permalink / raw) To: Shawn Landden; +Cc: git On Sun, Apr 14, 2019 at 04:48:53PM -0500, Shawn Landden wrote: > When I send patches I want them to have lots of context, but when just > looking at a diff, I can always open the file for context. Seems like a reasonable thing to want. You can already use "git format-patch -U20 ...". The usual advice for putting that in config is to define an alias wit your preferred option. I don't think it would be too big a problem for format-patch to learn some options to configure its diffs. We already have some options in format.* for various things. -Peff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: allow a differn't diff.context config for git format-patch 2019-04-23 3:06 ` Jeff King @ 2019-04-23 3:45 ` Junio C Hamano 2019-04-23 3:55 ` Jeff King 0 siblings, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2019-04-23 3:45 UTC (permalink / raw) To: Jeff King; +Cc: Shawn Landden, git Jeff King <peff@peff.net> writes: > ... I don't think it would be > too big a problem for format-patch to learn some options to configure > its diffs. We already have some options in format.* for various things. I am not sure; diff.context is rather special in that the variable behind it belongs to the diff layer and is set via diff_config that is called by diff_ui_config that in turn is called by log_config which is called by format_config. If we want to say "format.context" trumps "diff.config", then a new internal variable to receive the value of the variable must be invented and after the config gets read, cmd_format_patch() needs to do something silly like this: git_config(git_format_config, NULL); + if (format_context != -1) + diff_default_context = format_context; repo_init_revisions(the_repository, &rev, prefix); committing layering violation by exposing diff_default_context variable. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: allow a differn't diff.context config for git format-patch 2019-04-23 3:45 ` Junio C Hamano @ 2019-04-23 3:55 ` Jeff King 2019-05-12 13:21 ` Shawn Landden [not found] ` <CA+49okrzR7GAZCFDfaoJ9pvK+o=DVWoZ9vA6YL=u72s_rnjDxQ@mail.gmail.com> 0 siblings, 2 replies; 7+ messages in thread From: Jeff King @ 2019-04-23 3:55 UTC (permalink / raw) To: Junio C Hamano; +Cc: Shawn Landden, git On Tue, Apr 23, 2019 at 12:45:17PM +0900, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > ... I don't think it would be > > too big a problem for format-patch to learn some options to configure > > its diffs. We already have some options in format.* for various things. > > I am not sure; diff.context is rather special in that the variable > behind it belongs to the diff layer and is set via diff_config that > is called by diff_ui_config that in turn is called by log_config > which is called by format_config. > > If we want to say "format.context" trumps "diff.config", then a new > internal variable to receive the value of the variable must be > invented and after the config gets read, cmd_format_patch() needs to > do something silly like this: > > git_config(git_format_config, NULL); > + if (format_context != -1) > + diff_default_context = format_context; > repo_init_revisions(the_repository, &rev, prefix); > > committing layering violation by exposing diff_default_context > variable. After init_revisions(), we'll have called diff_setup(), which puts diff_context_default into revs->diffopt. So we still have to do the "if we have a format-specific value, then override..." conditional. But we can do it without touching the hidden variable: diff --git a/builtin/log.c b/builtin/log.c index 57869267d8..0de2ee0905 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1613,14 +1613,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) repo_init_revisions(the_repository, &rev, prefix); rev.commit_format = CMIT_FMT_EMAIL; rev.expand_tabs_in_log_default = 0; rev.verbose_header = 1; rev.diff = 1; rev.max_parents = 1; rev.diffopt.flags.recursive = 1; + if (format_context != -1) + rev.diffopt.context = format_context; rev.subject_prefix = fmt_patch_subject_prefix; memset(&s_r_opt, 0, sizeof(s_r_opt)); s_r_opt.def = "HEAD"; s_r_opt.revarg_opt = REVARG_COMMITTISH; if (default_attach) { rev.mime_boundary = default_attach; -Peff ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: allow a differn't diff.context config for git format-patch 2019-04-23 3:55 ` Jeff King @ 2019-05-12 13:21 ` Shawn Landden [not found] ` <CA+49okrzR7GAZCFDfaoJ9pvK+o=DVWoZ9vA6YL=u72s_rnjDxQ@mail.gmail.com> 1 sibling, 0 replies; 7+ messages in thread From: Shawn Landden @ 2019-05-12 13:21 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, git On Mon, Apr 22, 2019 at 10:55 PM Jeff King <peff@peff.net> wrote: > > On Tue, Apr 23, 2019 at 12:45:17PM +0900, Junio C Hamano wrote: > > > Jeff King <peff@peff.net> writes: > > > > > ... I don't think it would be > > > too big a problem for format-patch to learn some options to configure > > > its diffs. We already have some options in format.* for various things. > > > > I am not sure; diff.context is rather special in that the variable > > behind it belongs to the diff layer and is set via diff_config that > > is called by diff_ui_config that in turn is called by log_config > > which is called by format_config. > > > > If we want to say "format.context" trumps "diff.config", then a new > > internal variable to receive the value of the variable must be > > invented and after the config gets read, cmd_format_patch() needs to > > do something silly like this: > > > > git_config(git_format_config, NULL); > > + if (format_context != -1) > > + diff_default_context = format_context; > > repo_init_revisions(the_repository, &rev, prefix); > > > > committing layering violation by exposing diff_default_context > > variable. > > After init_revisions(), we'll have called diff_setup(), which puts > diff_context_default into revs->diffopt. So we still have to do the "if > we have a format-specific value, then override..." conditional. But we > can do it without touching the hidden variable: > > > Looks like this fell into the cracks. You guys know the code much better than me, but do I have to write a patch to make this happen? > diff --git a/builtin/log.c b/builtin/log.c > index 57869267d8..0de2ee0905 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1613,14 +1613,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > repo_init_revisions(the_repository, &rev, prefix); > rev.commit_format = CMIT_FMT_EMAIL; > rev.expand_tabs_in_log_default = 0; > rev.verbose_header = 1; > rev.diff = 1; > rev.max_parents = 1; > rev.diffopt.flags.recursive = 1; > + if (format_context != -1) > + rev.diffopt.context = format_context; > rev.subject_prefix = fmt_patch_subject_prefix; > memset(&s_r_opt, 0, sizeof(s_r_opt)); > s_r_opt.def = "HEAD"; > s_r_opt.revarg_opt = REVARG_COMMITTISH; > > if (default_attach) { > rev.mime_boundary = default_attach; > > -Peff ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CA+49okrzR7GAZCFDfaoJ9pvK+o=DVWoZ9vA6YL=u72s_rnjDxQ@mail.gmail.com>]
* Re: allow a differn't diff.context config for git format-patch [not found] ` <CA+49okrzR7GAZCFDfaoJ9pvK+o=DVWoZ9vA6YL=u72s_rnjDxQ@mail.gmail.com> @ 2019-05-14 9:54 ` Jeff King 2019-05-14 9:57 ` Jeff King 0 siblings, 1 reply; 7+ messages in thread From: Jeff King @ 2019-05-14 9:54 UTC (permalink / raw) To: Shawn Landden; +Cc: Junio C Hamano, git On Sun, May 12, 2019 at 07:29:42AM -0500, Shawn Landden wrote: > > After init_revisions(), we'll have called diff_setup(), which puts > > diff_context_default into revs->diffopt. So we still have to do the "if > > we have a format-specific value, then override..." conditional. But we > > can do it without touching the hidden variable: > > > Looks like this fell into the cracks. You guys know the code much better > than me, but do I have to write a patch to make this happen? I think there is more work to be done in actually parsing such a config option, plus adding documentation and tests. But more importantly, it needs somebody to champion the feature. I don't find it an unreasonable thing to want. But I'm not personally interested, nor am I completely sure that it wouldn't have any unwanted side effects. So ideally somebody would write the patch and a compelling commit message that explains why it is a good idea. -Peff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: allow a differn't diff.context config for git format-patch 2019-05-14 9:54 ` Jeff King @ 2019-05-14 9:57 ` Jeff King 0 siblings, 0 replies; 7+ messages in thread From: Jeff King @ 2019-05-14 9:57 UTC (permalink / raw) To: Shawn Landden; +Cc: Junio C Hamano, git On Tue, May 14, 2019 at 05:54:46AM -0400, Jeff King wrote: > On Sun, May 12, 2019 at 07:29:42AM -0500, Shawn Landden wrote: > > > > After init_revisions(), we'll have called diff_setup(), which puts > > > diff_context_default into revs->diffopt. So we still have to do the "if > > > we have a format-specific value, then override..." conditional. But we > > > can do it without touching the hidden variable: > > > > > Looks like this fell into the cracks. You guys know the code much better > > than me, but do I have to write a patch to make this happen? > > I think there is more work to be done in actually parsing such a config > option, plus adding documentation and tests. > > But more importantly, it needs somebody to champion the feature. I don't > find it an unreasonable thing to want. But I'm not personally > interested, nor am I completely sure that it wouldn't have any unwanted > side effects. > > So ideally somebody would write the patch and a compelling commit > message that explains why it is a good idea. By the way, if you (or anybody) is interested, looking at how signoff is implemented (try "git grep -i format.signoff") should get you most of the way there. -Peff ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-14 9:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-14 21:48 allow a differn't diff.context config for git format-patch Shawn Landden 2019-04-23 3:06 ` Jeff King 2019-04-23 3:45 ` Junio C Hamano 2019-04-23 3:55 ` Jeff King 2019-05-12 13:21 ` Shawn Landden [not found] ` <CA+49okrzR7GAZCFDfaoJ9pvK+o=DVWoZ9vA6YL=u72s_rnjDxQ@mail.gmail.com> 2019-05-14 9:54 ` Jeff King 2019-05-14 9:57 ` Jeff King
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.