[MERGE] enhanced mv command

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Marius Kruger wrote:
...

> If you don't like trivial improvements like this even if it is done in an
> organized fashion,
> I think the vcs and other tools are failing you.
> * Something which would be useful for me is to have a gui annotate, which
> could
>   drill down when you click on a line: Say the line's revisoin is 10, do a
> 'bzr annotate -r9'.
>   I.e re-annotate up to a revision before its current last change.

Well, we already have:

% bzr annotate -rX
# See the last-modified revision == Y
% bzr annotate -rbefore:Y

rinse and repeat

It might be nice to have that as a possibility for gannotate. It would
be *really* slick if it could do it based on change groups, with a
little triangle to expand a change group to its previous contents. But
Aaron pointed out why this is actually quite difficult, because of what
is actually happening to the texts.

> 
> I make a practice of diffing my changes, usually against the mainline,
>> so that I can catch and undo spurious changes.
> 
> 
> I also started to do that,  I also parse it through
> 1) add-trailing-white-space-detector,
>    (I should probably have a alter-trailing-white-space-detector.)
> 2) break-up-into-words | aspell list
>    (as I'm quite bad at spelling, but theres no tools to fix my grammar)
> 

Well, vim 7.0 now has built-in spell checking. But I haven't seen a way
to tell it to spell-check doc strings and comments, and not spell check
code.

...

> If I write a test without fixing it, the test will fail and thus (I think)
> you would
> not be able to merge it in, as it will start failing all over.
> What I was thinking is also providing a small python script which helps you
> remove
> all trailing white space in all the .py files.

You don't really need a python script. It is as simple as:

bzr ls --versioned --null | xargs -0 sed -i~ -e 's/\s\+$//'

(admittedly that doesn't look terribly simple, but I'm trying to say it
is a simple substitution regex s/\s\+$//)

The current problem is that in the 'rio' tests, there is trailing
whitespace that must be preserved. Though we could escape it to
something like: '\x20'. I think it is a bigger "bug" that trailing
whitespace is significant in RIO formats, but that is the way it was
written.

The reason is how we handle empty values:
    if value == '':
        result.append(tag + ': \n')
    elif '\n' in value:
        # don't want splitlines behaviour on empty lines
        val_lines = value.split('\n')
        result.append(tag + ': ' + val_lines[0].encode('utf-8') + '\n')
        for line in val_lines[1:]:
            result.append('\t' + line.encode('utf-8') + '\n')
    else:
        result.append(tag + ': ' + value.encode('utf-8') + '\n')

(I think the first case is actually redundant with the last, but that
might be okay).


> Then all diverging branches must just apply it to themselves first before
> attempting to merge back in. Maybe a special date must be set for it,
> for example: "All trailing white space will be removed on 2007-02-12
> and tests for this will be added, all bundles submitted after that must be
> cleaned up by the branch (using a script provided)."

I tend to go for something like this. And would even be willing to merge
and resolve conflicts for all of Aaron's branches, or anyone else who
might have long-term branches still outstanding.

I don't find merge conflicts of this sort to be that hard to go through,
and if we have transitioned away (and added tests) we won't introduce
new trailing whitespace, and thus shouldn't have this sort of problem in
the future.

I actually have a branch where I started doing some of this (including
writing the test). But at this point I would rather keep Aaron happy,
since he does a lot of good work.

...

>> >
>> >     ^^^ Is it possible to catch more specific errors here?  BzrError
>> and
>> >     OSError are very broad.
>> >
>> > (Again I didn't write this, so don't be too hard on me)
>> > I think its ok, as we just want to rollback and raise it again.
>> > It is done like that elsewere in the file too.
>>
>> Really, I don't see why anyone would do this.  Either you want to catch
>> *all* exceptions and roll back, or you want to catch specific
>> exceptions, and only roll back on those.  In particular, OSError does
>> not necessarily indicate that a rename failed.  You would have to wrap
>> the try/catch around osutils.rename in order to know whether the rename
>> failed.
> 
> 
> I don't really know what to do about this.
> 

I agree with Aaron here, either we want to catch all errors and rollback
with a blank "try/except:" or just not try to rollback the move.

...

>>
>> >  from bzrlib.patches import (PatchSyntax,
>> >                              PatchConflict,
>> >                              MalformedPatchHeader,
> 
>
> It will be a sweet exercise to write tests for these too.
> But then it also needs to be cleaned up everywhere.
>


...

> Another option would be to extend assertIs to take a
>> message, and add assertIsNot.
> 
> 
> Its up to you, but I prefer assertNone.

I think assertIsNot(None, something) and assertIs(None, something) is
better than a special case for 'assertNone'. Especially since the
resulting message can work just fine for None, and works fairly well for
other cases:

if actual is not expected:
  self.fail('Actual value %r is not %r' % (actual, expected))

Would give you the error "Actual value 'foo' is not None", which is
pretty much what you would put for:

if actual is not None:
  self.fail('Actual value %r is not None' % (actual,))


John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFp7TiJdeBCYSNAAMRAp5LAKCwSISglAbvXk8h/nQQxQcB1CnvAQCeOXUm
LQsBHVr/OAW0JjjiVqKJJ/M=
=wYzz
-----END PGP SIGNATURE-----