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: Mon, 12 Apr 2010 20:49:05 -0600

On Mon, Apr 12, 2010 at 4:06 PM, Thomas Keller <address@hidden> wrote:
No, I just tried it out and it already looks much nicer, though I still
miss the "end" of the editable changelog area somewhat - maybe this is
just me who looks for some visual separator.

One minor peeve I have with the current editor (on mainline) is that you don't really want to put a blank line after the message or it will be displayed as 2 blank lines after the revision is committed. If I reformat the paragraph I'm typing in emacs with fill-paragraph and there is no blank line following my message it pulls in all the MTN: prefixed lines and makes a bit of a mess. With the new format I was sort of relying on the fact that there would be a blank line following the message to prevent fill-paragraph from being overly greedy. Maybe this delimiter is too subtle though? Presumably someone could write a monotone-commit-mode for emacs and something similar for vim that would allow the editor to prevent you from editing things you're not supposed to.
 
I have to admit I used the comment command the first time today. I now
know what you mean, mtn comment REV fires up an editor with the
possibility to add multi-line text. While you can add linebreaks in
other certs as well, its usually a bit harder to do so and people don't
trap into that. I initially thought that a comment cert's value could
simply appended to the header section in mtn log or similar, but it
makes more sense to deal with it just like with normal changelog certs.

See 'mtn log -r 0f1782f7e2348f991a0b8eeac03c45a72c8633a2' for a revision with a few comment certs. I think the current format does a reasonable job there.

###########
Enter a description of this change following the ChangeLog line.
The values of Author, Date and Branch may be modified as required.
Any other modifications will will cause the commit to fail.
*** REMOVE THIS LINE TO CANCEL THE COMMIT ***
----------------------------------------------------------------------
Author:   address@hidden
Date:     12.04.2010 23:44:27
Branch:   biz.thomaskeller.test3

ChangeLog:


----------------------------------------------------------------------
Parent: ace2791b0b3df530b449802ce82fd8d731a466f1

 patched  foo

############

What has changed? Parent has been moved down into the ChangeSet section.

I had considered this a while ago but then forgot about the idea so thanks for reminding me. We currently have the Parent listed twice, once in the header and a second time before the changeset. Maybe we should drop the one in the header and only include it with the changeset, DRY and all that.

I wouldn't call it ChangeSet but Parent, simply because it is the parent

Interestingly (maybe) in the code what is being displayed is exactly the changeset from the revision which is where the name came from.
 
we denote with the revision. Revision is gone from the commit header as
well - is there any use case displaying this for uncommitted revisions?

None that I can think of... I was looking for consistency but I'm fine with dropping the Revision line from commit and status. Any objection to dropping (or moving) the Parent lines down with their changeset details?

I would probably not change too much in mtn log, i.e. separators for
marking an editable section are not needed here. I'd also keep most of
the structure as it is now - also because I think we still have a couple
of users which read in mtn log and parse it somehow directly because of
the missing automate functionality for that. I know that this is a bit
against your aim to provide a single way of displaying cert information,
but maybe its worth to not do it exactly the same...?

I was hoping/expecting to hear from anyone who might be parsing the log output if they were concerned with the changes being proposed. By unifying the formats I was hoping that people could see what their revision would look like before they've committed it so that there aren't any surprises introduced by the different formats. The mess of functions to display certs in log is mostly gone on this branch and that's one part of the change that I'd like to keep. ;)

Hrm, no, probably not that far. Ok, forget the RFC :) If you think its
easy enough to add some basic custom cert adding, then do it, otherwise
we wait for a feature request...

I'm punting for now. We can add this later if it becomes important.
 
> 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
>>
>> ----------------------------------------------------------------------

Status explicitly outputs a blank line after the old/new branch details which can be changed if we want. I like the idea of adding this information to the instructions for commit if/when the branch is changing.  A few extra lines there would make an accidental branch change quite hard to miss.

Ok, thanks for your continued work and for standing all the discussion

No problem. I was entirely expecting there to be some debate before we might agree that landing this was a good idea. I'm even a bit surprised that there hasn't been violent objections.

If anyone else has something they'd like to add please do, particularly anyone who's parsing the output from 'mtn log' and who would be really disappointed if the format changes and breaks their scripts!

Cheers,
Derek


reply via email to

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