git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] wt-status: use rename settings from init_diff_ui_defaults

From
Matthieu Moy <git@matthieu-moy.fr>
Date
May 1, 2018, 12:23 UTC
Message-ID
<1652522802.213664.1525177431907.JavaMail.zimbra@matthieu-moy.fr>
In-Reply-To
<20180501114316.GB13919@esm>
"Eckhard Maaß" <eckhard.s.maass@googlemail.com>:
> On Tue, May 01, 2018 at 01:09:06PM +0200, Matthieu Moy wrote:
> > That init_diff_ui_defaults() should indeed have been before
> > git_config() from the beginning. My bad, I'm the one who
> > misplaced it apparently :-(.
> Should I have done this "bug fix" in a separate commit or mention it in
> the commit message?

I'm fine with it as-is. Before your "fix", the config was ignored because overwritten by init_diff_ui_defaults() after reading the config, so effect of your change is indeed what the commit message describes.

I'm often thinking aloud while reviewing, don't take my comments as objections.

> > This "break_opt = 0" deserves a mention in the commit message IMHO.
> > I'm not 100% sure it's a good change actually.
> Hm, what problems do you see here?

I don't see any "problem", I *think* your change is good, but I can't fully convince myself that it is without further explanation.

Unlike the other two, this option has no corresponding configuration variable, so the "let the config" argument doesn't apply. For "git status", there's actually not even a command-line option. So, this assignment removed, there's no way in the user-interface to re-enable the previous behavior. *If* there was a good reason to get "break_opt = 0", then your patch is breaking it.

Unfortunately, the commit introducing it doesn't help much: f714fb8 (Enable rewrite as well as rename detection in git-status, 2007-12-02) is just a one-liner message with a one-liner patch.

But actually, I never used -B/--break-rewrites, and writting this message I tried to get a case where -B would make a difference and I'm not even able to find one. So, as someone who never understood the real point of -B, I'm not sure I'm qualified to juge on what's the best default ;-).

-- 
Matthieu Moy
https://matthieu-moy.fr/
Previous: Eckhard MaaßNext: Elijah Newren
Message 6 of 14 in “wt-status: use rename settings from init_diff_ui_defaults”
  1. wt-status: use rename settings from init_diff_ui_defaultsEckhard S. Maaß, May 1, 2018
  2. Ævar Arnfjörð BjarmasonMay 1, 2018
  3. Eckhard MaaßMay 1, 2018
  4. Matthieu MoyMay 1, 2018
  5. Eckhard MaaßMay 1, 2018
  6. Matthieu MoyMay 1, 2018
  7. Elijah NewrenMay 1, 2018
  8. Junio C HamanoMay 1, 2018
  9. Elijah NewrenMay 2, 2018
  10. Ben PeartMay 2, 2018
  11. Eckhard MaaßMay 3, 2018
  12. wt-status: use settings from git_diff_ui_configEckhard S. Maaß, May 4, 2018
  13. Elijah NewrenMay 4, 2018
  14. Eckhard MaaßMay 1, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.