[pqm-submit:MERGE] Remove default --message

[James Henstridge]

On 18/03/2008, John Arbash Meinel <john at arbash-meinel.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>
>  James Henstridge wrote:
>
> > On 18/03/2008, John Arbash Meinel <john at arbash-meinel.com> wrote:
>  >>  > That does seem like a bug.  So you're changing it to do open() rather
>  >>  > than open_containing() if a path is provided?
>  >>  >
>  >>
>  >>
>  >> Well, it already had a check for "if relpath and not tree", I just
>  >>  changed it to "if relpath and not tree and location != '.'".
>  >>
>  >>  I believe what people want is the ability to be in a subdir and do "bzr
>  >>  pqm-submit". I'm fine with that. What we are turning off is any case of
>  >>  "bzr pqm-submit 'foo'" that doesn't directly refer to a branch.
>  >
>  > Right.  With no argument, I'd expect the command to act on a the
>  > current branch (so do open_containing('.')).  If I pass a branch
>  > location though, I'd expect it to do open(location) instead.
>  >
>  > This seems to be the heuristic that "bzr check" uses, but it does not
>  > seem consistent between commands :(
>  >
>  >
>  >>  So... effectively plain .open() but I just worked it into the way we are
>  >>  doing it now.
>  >>
>  >>  It is a bit complex because we want to open a working tree if it is
>  >>  available so that we can check it is clean. The way I would usually do
>  >>  that is:
>  >
>  > Did you mean to write something extra here?
>
>
> I was going to start writing something, but then I didn't bother. I
>  might be the only one who writes it this way, as Aaron seems to prefer
>  open_containing all the time.

The fact that you can successfully open_containing() a non-existent
path (as spiv discovered) makes this sound a bit dangerous.


>  Generally something like:
>
>  if location is None:
>   try:
>     tree = WT.open_containing('.')
>   except ...:
>     tree = None
>     b = Branch.open('.')
>   else:
>     b = tree.branch
>  else:
>   try:
>     tree = WT.open(location)
>   except ...:
>     tree = None
>     b = Branch.open(location)
>   else:
>     b = tree.b
>
>  It is a bit clumsy, and gets worse if you need an optional repository as
>  well. But except for commands like "bzr cat" I feel like you don't
>  really want Branch.open_containing()

That logic seems sensible to me.  This does sound like the sort of
logic that should be common to all relevant commands, so I wonder if
it should be encapsulated in a bzrlib function?

James.