Re: Discussion for interactive --patch commands to get --unified support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> If it is only to specify how many context lines to ask for the diff
> machinery when preparing the initial patch that is presented in the
> "add -p" UI, it should be fairly easy.

This is the intention, I personally don't have a need for changing
hunk context *during* an interactive add and as you say, this looks to
be a big task.

> I would expect that development of such a feature would progress
> roughly in the following order.

As someone who has never contributed to (or even very familiar with)
the codebase, this is very helpful and I really appreciate it!

> In my opinion it would be fine to respect diff.context (and probably
> diff.interhunkcontext[1]) by default.

I think this could work nicely and agree with this approach.

> Looking at git_diff_ui_config(), which are all the options read by
> porcelain git-diff but not by plumbing git-diff-files, etc, there
> may be other config in the same boat. E.g., I'd guess that people
> with diff.colormoved set would appreciate seeing that effect in the
> colorized versions we show.

Agreed. As you mention it, I recently discovered `diff.colormoved` and
I'd welcome this to the `--patch` functionality, although I also agree
that it would make sense to take things one step at a time and it's
not necessary to batch anything else into this change.

With all that being said, I propose the following:
- inheriting `diff.context` and `diff.interhunkcontext` as the
defaults for the various interactive/patch commands
- be able to override these defaults on the command line with
`-U<n>/--unified=<n>` and `--inter-hunk-context=<n>` respectively

These would cover both a "permanent" config that you wouldn't need to
specify every time when writing the command/s out, but also would
provide a way to override the individual built-ins as/when you want.

You guys obviously have better knowledge and experience with git
development so it would be great to hear your feedback on this.

Thanks!

On Tue, 29 Apr 2025 at 23:09, Jeff King <peff@xxxxxxxx> wrote:
>
> On Tue, Apr 29, 2025 at 10:16:15AM +0100, Leon Michalak wrote:
>
> > - make `diff.context` setting extend to the interactive patch commands
> > (not sure how a change like this would be welcomed considering it
> > could change users command outputs seemingly out of nowhere)
> > - add an `interactive.context` setting that would work like the
> > existing `diff.context` setting but apply only to the interactive
> > patch commands
>
> In my opinion it would be fine to respect diff.context (and probably
> diff.interhunkcontext[1]) by default. Though it does change the command
> output, the interactive output is by definition user-facing, so we
> shouldn't be breaking scripts. And we already respect other porcelain
> level config like colorizing.
>
> I think the only reason we don't already do so is that the interactive
> code is built around the plumbing commands, which conservatively avoid
> various config options. So the calling code has to explicitly check the
> config itself.
>
> -Peff
>
> [1] Looking at git_diff_ui_config(), which are all the options read by
>     porcelain git-diff but not by plumbing git-diff-files, etc, there
>     may be other config in the same boat. E.g., I'd guess that people
>     with diff.colormoved set would appreciate seeing that effect in the
>     colorized versions we show. But I think it is OK to just consider
>     diff.context for now, and see if anybody ever cares enough about
>     other options to look into them.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux