Re: [PATCH v8] status: modernize git-status "slow untracked files" advice

[Rudy Rigot <rudy.rigot@gmail.com>]

Thanks for the amazing feedback, and sorry for the delay.


> "time" I can sort of understand ("can reduce the time taken to
> enumerate untracked files" is how I may phrase it, though), but
> what did you want to say with "size"?

This bit was provided by a past reviewer so I hope I don't
misrepresent it, but I think the idea was to convey that the reason
it's faster is because the part of the codebase it will do active work
on is smaller. I don't think it provides compellingly more information
for end users, compared to just mentioning time, so I'll simplify with
your proposal here.


> Additionally (read: you do not _have_ to do this to make this topic
> acceptable, but it probably is worth thinking about), if we need to
> introduce a new helper function uf_was_slow() anyway, a much better
> change may be to make the 2 seconds cut-off configurable, than
> inventing GIT_TEST_UF_DETAIL_WARNING used only for tests.

I agree it would be an improvement, I'm going to try to do this. This
doesn't feel like a much more involved change compared to your
alternative suggestion of setting a hardcoded test value for
`s->untracked_in_ms`, so it feels like there's not much to lose from
doing it this way, while users would gain some nice configurability.

For transparency, my intuition is that I'm not sure there will be use
cases where the config will be meaningfully leveraged by users. My gut
is that the current cut-off time is an arbitrary UX perception
cut-off, so I'm not sure it would need to be different depending on
given repo situations. But I'm also very aware that I could be wrong,
and this could open use cases that I'm not thinking about. And to your
point, it would be sensible to use it as a test input anyway, so we
might as well make it a user-facing tweak; so, I'm on board.


> Wouldn't it be redacted into "It took 9X seconds to enumerate"?
> It probably does not happen, only because you are forcing the code
> to pretend that it took 2.001 seconds or something, I suspect.

Yup, you guessed right. I'll be sure to change the regular expression
to be resilient to double-digit times.


> Also, what do you need /g modifier in "sed" script for?  I do not
> think we give more than one such number in the message we are
> testing.

Indeed, it's not useful to anything, and shouldn't be there, I'll fix this too.


All of the other comments are crystal clear, and I intend to implement
them exactly as advised. You can expect a new patch this week, maybe
even today.


Thanks a lot again!