On Tue, Apr 20, 2010 at 3:57 PM, Thomas Keller
<address@hidden> wrote:
In the meantime what I would really really want to get rid of are
boolean function arguments - so maybe you could start and make the code
a little easier to read by passing a more describable enum value...?
Agreed. I've replaced the bool with an enum.
> I tend to agree here and I've reverted it to use the "Changes against
> parent..." however I'm now wondering whether displaying the branch names
> associated with each parent in this section might be useful which makes me
> wonder again about using Parent: ... and Branch: headers here.
This might surely make sense, but where do we stop? If we start listing
cert values of *parents* we might start listing the parents of these
parents and their certs and... endless loop :)
No, I think the parent revision id is enough, really. Everything else
should be part of that revision's log output.
Heh, ok. Nothing else to do here then.
Now this is something which I'd slightly change - I'd love to see this
above the editable area - because its important and might otherwise get
easily forgotten, so something like:
I'm not particularly fond of the old branch value in the editable area only because it clutters up the otherwise nicely aligned headers. I'm also not sure including the notice that the branch is changing before or after the editable area makes much difference. It may scroll off the top of a terminal if it's too early so it may be better after the editable values. I've left these where they were in the last iteration but I've emphasized the branch change message so it has a better chance of being noticed.
> You can set EDITOR='emacsclient +15' to get what you want but this is
> probably not what you want EDITOR set to in general. We'd need to do some
> more work in the edit_comment hook to get this right. This would be nice to
> do but I don't think it's critical for getting this branch finished.
Instead of giving the editor a "jump point" (which could be error-prone
f.e. if you think about i18n) I'd try to decrease the verbosity of the
entry line mainly by removing the warning in line three:
Done.
I'd like to preserve the option of telling the editor to jump though, and since that's currently done in EDITOR we can all set it to the appropriate value for our translations so that shouldn't be a real problem.
And of course I'd remove Revision: and Parent: from the editable area to
make it even less verbose, but we've had this discussion before and as I
said I'm happy already that my separators made it in ;)
Yes, you're arguing for removing these non-editable things and including the non-editable old branch value. I'm arguing for keeping these and not including the old branch value. It's pretty arbitrary either way there's no doubt.
My rationale for leaving the Revision and Parent headers in is that without them the nicely aligned headers look a bit odd because they have extra alignment whitespace that is only relevant when those headers do appear in the output from log and status and I'd like to keep the display from these three commands as similar as possible.
I think we definitely want to have it for 0.48. And I'm also voting for
not discussing this useful feature to death (because then it won't get
included at all) - so I'm shutting up now :)
I appreciate the feedback all the same. All that remains is to update the manual to include some examples of the new editor display. I should have this done some time this week.
Sorry it took me so long to get to this.
Cheers,
Derek