Re: [PATCHv4 1/3] wt-status.*: better advices for git status added

Junio C Hamano [off-list ref] writes:

And instead, hide the above new lines behind advice.statusHints,
without introducing advice.statusHelp.  As to the global code
structure, it probably would make more sense to:

  - rename wt_status_evaluate_state() to wt_status_print_state();

  - rename the various "print help information for this state" functions
    that are called in the above if/else/... cascade to merge_in_progress_show()
    etc.

  - move the above if/else/... cascade to the end of
    wt_status_print_state(), which would make the above part more
    like:

	 wt_status_print()
         {
		if (s->branch) {
                	...
		}
	+	wt_status_print_state(s);
		if (s->is_initial) {
			...

  - at the beginning of wt_status_print_state(), check advice.statusHints
    and return without doing anything if the user does not want hints.

Otherwise, overall the patch is getting better looking.

Thanks for a pleasant read.

After reading the current code, I changed my mind slightly.

The way the new code tries to separate the state information
(i.e. statement of the fact it found) and the advice messages
(i.e. what could be done next) is indeed in line with what the
advice.statusHints configuration is used in the existing code.  We
unconditionally show what we find to the user, and we give help by
default but let the user decline.

So an updated suggestion would be to still move the whole thing to
wt_status_print_state(), but not check advice.statusHints at the
beginning of the function (as we will show what we find even when
the user declines help text) and do the discovery, and hide the help
messages (i.e. "do this to proceed") behind advice.statusHints,
without introducing advice.statusHelp.

As I pointed out in my review, there were some places in the patch
that gave help text unconditionally, which needs to be corrected.