Re: [PATCH 3/4] push: make non-fast-forward help message configurable

Jeff King [off-list ref] writes:

On Mon, Sep 07, 2009 at 09:44:57AM +0900, Nanako Shiraishi wrote:

quoted

You may be fixated at the sha1 part of the message when you find this
message annoying, but I disagree strongly. I always appreciate the
assurance this message gives me that I counted the number of commits
correctly, whether I say HEAD^^^^ or HEAD~7.

Let me add a "me too" to Nanako's comments.

I guess it depends on one's workflow. If you usually cut-and-paste
sha1's, then the message is superfluous, while if you usually use
magic revspec, it is useful.

So, it's probably a good idea to make this configurable.

So really they are two different conceptual types of message. And while
I have no problem with an argument of "I _personally_ find this clutter
and would like to configure it off", I don't think such an option should
go under "advice.*". My patch had "message.all" (which will become
"advice.all")

To me, this is an argument in favor of keeping "message", to allow the
same mechanism for these different types of messages.

But I think the individual message.* should not be just true/false
switch, but could be always/auto/never :

- always: show the message, regardless of message.all
- auto (the default): rely on message.all to decide whether to show
  the message
- never: never show it.

So you could say "message.all = false" and "message.resetShowsNewHead
= always".

But maybe that's just overkill, dunno...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

On Mon, Sep 07, 2009 at 10:34:25AM +0200, Matthieu Moy wrote:

quoted

So really they are two different conceptual types of message. And while
I have no problem with an argument of "I _personally_ find this clutter
and would like to configure it off", I don't think such an option should
go under "advice.*". My patch had "message.all" (which will become
"advice.all")

To me, this is an argument in favor of keeping "message", to allow the
same mechanism for these different types of messages.

But I think the individual message.* should not be just true/false
switch, but could be always/auto/never :

- always: show the message, regardless of message.all
- auto (the default): rely on message.all to decide whether to show
  the message
- never: never show it.

So you could say "message.all = false" and "message.resetShowsNewHead
= always".

But maybe that's just overkill, dunno...

I'm not sure it solves the problem. The point of "message.all" was to
easily say "I'm an expert, so turn off useless advice". But now I would
have to manually re-enable any messages that I _do_ want to see. And of
course I don't see them to know that I want them, so I have to read
through the config documentation and decide on each one.

At that point, why not just get rid of "message.all" and simply say
"manually turn off the messages you don't like". Then the user can
either go through the config manually as above, or they can wait until
they become annoyed with a particular message and turn it off (and
hopefully our naming is good enough that they can easily figure out
which one it was :) ).

So I think "be verbose, but let the user quiet us" is probably
better than "be quiet, but let the user make us louder", because it is
easier to discover verbose things. Which implies to me that
"message.all", if it exists at all, should be limited in scope to just
advice.

In fact, you could mix many types in the message.* hierarchy and simply
call the umbrella variable message.advice. But that is semantically
equivalent to having advice.{all,*} and other_type_of_message.{all,*}.

-Peff

On 09/07/2009 01:54 AM, Jeff King wrote:

At that point, why not just get rid of "message.all" and simply say
"manually turn off the messages you don't like". Then the user can
either go through the config manually as above, or they can wait until
they become annoyed with a particular message and turn it off (and
hopefully our naming is good enough that they can easily figure out
which one it was :) ).

So I think "be verbose, but let the user quiet us" is probably
better than "be quiet, but let the user make us louder", because it is
easier to discover verbose things. Which implies to me that
"message.all", if it exists at all, should be limited in scope to just
advice.

That seems like the most sane solution, given that it may not be obvious 
to the user what messages he/she may be missing if "expert" mode is on, 
and also from a coding perspective.

I know that by default I'd like to see new messages, and in case I'm 
doing something adventurous I'd like to see a message I may not have 
seen before. If I had turned off all messages because I got sick of 
seeing one or two in particular, then I'd never know.

Also, this thread started due to people's dislike of one particular 
message, so I think it's likely that in general someone would really 
only want to turn off at most a handful of messages, which again points 
to turning them off individually as being the best and simplest solution.

P.S. I never really introduced myself to the list... Uri Okrent here 
from L.A. Keep up the great work everyone!
-- 
    Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/

On Tue, Sep 08, 2009 at 11:51:21AM -0700, Uri Okrent wrote:

I know that by default I'd like to see new messages, and in case I'm
doing something adventurous I'd like to see a message I may not have
seen before. If I had turned off all messages because I got sick of
seeing one or two in particular, then I'd never know.

Yeah, I think the 'advice.all' option, while clever, is not really
helping anyone. In my revised series, I am just omitting entirely (but
the code is still structured that adding it later would be very easy if
we change our minds).

P.S. I never really introduced myself to the list... Uri Okrent here
from L.A. Keep up the great work everyone!

Welcome. :)

-Peff