Where does self.message get set on the Revision object?

[John Szakmeister]

On Tue, Jun 2, 2009 at 12:50 PM, John Arbash Meinel
<john at arbash-meinel.com> wrote:
[snip]
> Looking at the code, in xml8.py it does:
>
>  rev.message = elt.findtext('message')
>
> If there was no 'message' property on the value, then this would return
> None.

Sorry, I meant to say that I found where this was being set (in a
couple different formats).

> I would also mention that you should try "bzr log --line" which uses
> get_summary() as well.

Heh.  I could have sworn I tried that, and it didn't traceback.
Perhaps it was --short that I tried (although I'm pretty sure I tried
both).  At any rate, I just tried again, and it does indeed traceback.

> The log formatters seem to be doing:
>        if not revision.rev.message:
>            to_file.write(indent + '  (no message)\n')
>        else:
> Though I notice that the "LineLogFormatter" doesn't do such a check.
>
> Note that "if not revision.rev.message:" will also handle the case where
> the commit message is the empty string. (So both None and '' are handled.)

*nod*

> Now, I don't *know* what we want to display for when there is no
> revision message. I can say that:
>
>  bzr log --line
>
> Will show nothing for a revision with no message.

Just for clarification: it shows the revision number, date and author.
 You just don't see anything for the log message portion of the line.

> Digging a bit deeper, I don't think that a properly formatted Revision
> that is serialized down to disk, and then back up will have None for
> message. I poked around, and doing msg = SubElement(root, 'message'),
> will *always* create an element in the final string, and when you do
> findtext('message') you get back the empty string (''), rather than None.

Should the revision be properly-formatted before it hits the disk?  I
guess what I'm looking for is what is acceptable, and when.

> So I don't really care if you add:
>
>    def get_summary(self):
>        """Get the first line of the log message for this revision.
>        """
>        if self.message is None:
>            return ''
>        return self.message.lstrip().split('\n', 1)[0]

Do we want to take the approach that's used elsewhere and return '(no
message)'?  There seems to be precedent for that in the code base
already.

> But I'll comment that this Revision object has probably not been
> serialized to disk. And my bet is that it is generated from something
> like bzr-svn.

I believe I stated that initially, but yes, it's coming from bzr-svn.
I ran the client directly against the Subversion repository (*the*
Subversion repository), and ran into a commit that didn't have a log
message.  If it's bzr-svn's fault (and it sounds like you're hinting
that it is), then I can look to make the change there so that it
provides a default empty message.  If it's truly expected that
self.message can be None, get_summary() should definitely be updated,
and depending on it's output, qbzr might need to be fixed to.  I'm
just trying to figure out where I need to focus my energy. :-)

I'll definitely send submit a patch and a test for get_summary().  But
if anyone knows the answer as to whether it's acceptable for .message
to be None, I'll submit a patch to bzr-svn as well, if it's considered
to be broken in that respect.

Thanks again!

-John