monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Monotone-devel] the changelog editor branch is ready for review


From: Derek Scherger
Subject: Re: [Monotone-devel] the changelog editor branch is ready for review
Date: Sun, 11 Apr 2010 20:09:12 -0600

On Sun, Apr 11, 2010 at 12:38 PM, Thomas Keller <address@hidden> wrote:
> If there is text in _MTN/log then it is displayed in the changelog section.

Ah, didn't thought of this - nice!

> We could choose to omit this section unless there is something in _MTN/log.

Yes, I'd say so.

Done.
 
> I've been wondering about a line following the instructions like:
>
> *** REMOVE THIS LINE TO CANCEL THE COMMIT ***

I've also cleaned up the instructions a bit and added this explicit cancel line.
 
Usually you can cancel the commit simply by not entering a commit
message. I'm not sure we need to have another way of cancelling a commit

If there is text in _MTN/log then there will be a commit message that you need to remove. The original text will be preserved in _MTN/log but I only know that from looking at the code. I would expect people to be reluctant to remove their entire message to cancel the commit and hope that their message was preserved somewhere.
 
The problem I have with %F is that it simply expands to the ISO Y-m-d
format which is not at all localized. Maybe we misunderstood each other
here, I thought ahead already and headed towards condition-like code
which sets %F in some locales and %x in others... this was the thing I
wanted to prevent.

No worries. I've left the default formats as they are on mainline and they work fine here, as long as the dates are between 1969 and 2068.
 
> Trimming is probably required if we choose to left align the headers anyway
> so I should probably just do that.

Right.

Done.
 
The right-aligned version seems to be the better choice.

Oops. I had the left aligned version done before your message. Let me know if you think we should change it. I have a very slight preference for the left aligned version now. In emacs shell-mode there is some colorization done by default and the left aligned headers are colored differently that other output lines. This doesn't seem to work with the right aligned versions. I'm not stuck on this and will change if you have a strong preference the other way.

Hrm, I haven't thought of the "comment" cert at all, is this a much used
feature after all? If not I'd probably don't care about it and would
display it like any other cert value we have.

I'm not quite sure what you mean by this. Comment certs are much like changelog certs, for the few that appear in the monotone database anyway and are quite likely to be multi-line comments so displaying them like the Date/Author/Branch certs doesn't make a lot of sense.
 
The log output already uses "---" as revision separators, so it might be
better to find a different separator for this then.

Wrt the double comment / changelog values - I solved this in recent
guitone versions twofold: At first I group a list of cert pairs by
signer, so its clearer which set of certs have been added by which
signer (while this of course gives the user no temporal or any other
information, we'd need the long discussed cert flag day for that
purpose). Secondly if there is more than one changelog cert (and I only
handle changelog certs special here, completly forgot about comment
certs) from one signer, I group both under one "Changelog" entry and
separate both by a horizontal line - pretty much what you propose here.

What log will do at the moment is produce multiple ChangeLog: entries with the header preceeding each one.
 
Right, there is only one small nitpick I have with a syntax like
"Changes:" - this pretty much then looks like a cert and in total like
the continuation of the header section. To make it clearer that this is
not the case I thought adding another separator line might be a good
idea, but I'm open for other ideas here.

I'm not sure that representing the changes similar to the certs is good or bad. The latest version is using "ChangeSet:" as the header for these summaries but I'm still open to suggestions.

I think we shouldn't make it too smart - it should be enough if it looks
for the four most used certs - Date, Author, Branch and Changelog - and
checks their syntax and single existance (to prevent the buffer
duplication issue). Then, if it finds other Key: Value pairs in the
header area, it should probably just read and add them as additional
certs. Maybe its even a good idea to follow some of the basic rules of
RFC2822 here, esp. when it comes to newline handling? (Of course we do
not expect full CRLF's and 7bit ascii here... but basically giving the
user something which looks and reacts familiar.)

At the moment I'd rather not get too far into this. Would we read a Foobar: header and generate a "foobar" cert after converting the header to lowercase?
 
On a completly unrelated note: one thing I haven't had in mind yet is
how the new functionality reacts on branch changing - I've edited
_MTN/options and set a new branch. mtn status told me that nicely
(though I wonder where the additional newline comes from): 

I'll have a look at this and see where the extra newline is coming from.
 
#########
mtn: warning: This revision will create a new branch
Old Branch: biz.thomaskeller.test
New Branch: biz.thomaskeller.test3

----------------------------------------------------------------------
Revision: c656088d9d89cf47a9351af9575709ec8ef8f220
Parent: 9efdf632e0fb8e149b47d3c23fe03b74f54ab861
Author: address@hidden
Date: 11.04.2010 20:34:57
Branch: biz.thomaskeller.test3
ChangeLog:



Changes against parent 9efdf632e0fb8e149b47d3c23fe03b74f54ab861

 patched  foo
#########

but mtn commit did not tell me that:

Since we have "pseudo" headers like Revision: and Parent: already, maybe
it might be a good idea to leave the warning on top for both, status and
commit, but move the "old branch" / "new branch" info into the header
section...?

Off the top of my head I don't have any great ideas here either. I'll play around with it a bit and see what I can come up with.

Cheers,
Derek




reply via email to

[Prev in Thread] Current Thread [Next in Thread]