traceback caused by difflimit = 0

As requested by James Westby, I've set PDB_BZR=1 in the env and committed again, to check the type of attachment_text:

(Pdb) type(attachment_text)
<type 'NoneType'>

Could it be that I've misunderstood how 'post_commit_difflimit = 0' is supposed to work?

If difflimit == 0 then it returns None, rather than the "diff too large"
string returned if the value is > 0 and the diff is larger than that. Either
a string should be returned, or the None value should be handled.

However, Steve said that he thought 0 would mean no-limit. I think
either that, or using -1 to mean this would be a good idea.

Thanks,

James

On Tue, 2008-07-22 at 18:35 +0000, James Westby wrote:
> If difflimit == 0 then it returns None, rather than the "diff too large"
> string returned if the value is > 0 and the diff is larger than that. Either
> a string should be returned, or the None value should be handled.
>
> However, Steve said that he thought 0 would mean no-limit. I think
> either that, or using -1 to mean this would be a good idea.

This is a regression - the docs say use 0 to disable the feature.

-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

On Wed, Jul 23, 2008 at 09:17:01PM -0000, Robert Collins wrote:
> On Tue, 2008-07-22 at 18:35 +0000, James Westby wrote:
> > If difflimit == 0 then it returns None, rather than the "diff too large"
> > string returned if the value is > 0 and the diff is larger than that. Either
> > a string should be returned, or the None value should be handled.

> > However, Steve said that he thought 0 would mean no-limit. I think
> > either that, or using -1 to mean this would be a good idea.

> This is a regression - the docs say use 0 to disable the feature.

FWIW, when I read the docs I understood "the feature" as "the feature of
limiting message size" rather than "the feature of sending mail"; it seemed
obvious to me that there were easier ways to disable sending mail, like by
not configuring a to: address.

So it would be nice if the docs could be clarified too.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

On Wed, 2008-07-23 at 21:45 +0000, Steve Langasek wrote:
>
>
> FWIW, when I read the docs I understood "the feature" as "the feature
> of
> limiting message size" rather than "the feature of sending mail"; it
> seemed
> obvious to me that there were easier ways to disable sending mail,
> like by
> not configuring a to: address.
>
> So it would be nice if the docs could be clarified too.

0 is meant to disable "the feature of limiting message size".

Ack on clarification opportunity though.

-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

I'm having a similar issue, and like the above suggestion, I've set the limit to -1.

File "/usr/lib/python2.5/site-packages/bzrlib/plugins/email/__init__.py", line 98, in branch_post_change_hook
    params.branch.get_config(), local_branch=None, op='change').send_maybe()
  File "/usr/lib/python2.5/site-packages/bzrlib/plugins/email/emailer.py", line 248, in send_maybe
    self.send()
  File "/usr/lib/python2.5/site-packages/bzrlib/plugins/email/emailer.py", line 185, in send
    self._send_using_process()
  File "/usr/lib/python2.5/site-packages/bzrlib/plugins/email/emailer.py", line 198, in _send_using_process
    message = self.body().encode('utf8') + self.get_diff()
TypeError: cannot concatenate 'str' and 'NoneType' objects

Just mentioning that we just encountered this. And we actually wanted the ability to disable showing the diff. Mostly because the people here want to see when trunk changes, but they don't care about the specifics of that change.