On Tue, Apr 20, 2010 at 3:57 PM, Thomas Keller <m...@thomaskeller.biz> 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
> information in the editable area and the hint that the branch changes
> 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
_______________________________________________
Monotone-devel mailing list
Monotone-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/monotone-devel

Reply via email to