BTW, I noticed that stgit tends to use space around the = in function keyword arguments. PEP8 says there should be no space there. Is the discrepancy intentional?
On Fri, Apr 4, 2014 at 4:06 AM, Karl Wiberg <[email protected]> wrote: > On Thu, Apr 3, 2014 at 6:54 AM, Erik Carstensen <[email protected]> > wrote: > > > test_expect_success \ > > 'No patch' \ > > '! stg move --from foo' > > * It's possible to write tests using far fewer backslashes. See > e.g. t/t3300-edit.sh. > Fixed > * Use general_error, command_error, or conflict from test-lib.sh > instead of !. Thanks! I also introduced a new function "expect_output". It makes the test more readable, and gives you a hint upon failure. * So moving a patch to the same branch will pop it if necessary, and > give it a new autogenerated name? Wouldn't it make more sense to > just complain and do nothing? > I was just checking that it doesn't do anything really bad; I thought it was easier not to care much about it. But considering your remark on -f below, it's easier to forbid it. Done now. > usage = ['DEST_BRANCH <patch1> [<patch2>] [<patch3>..<patch4>]'] > > description = """ > > Delete the given patches from the current branch, and add them unapplied > to > > DEST_BRANCH.""" > Fixed > > def find_unique_name(oldname, blacklist): > > # TODO: is this the right place? > > No, you can delete it because you never call it. But if you did want > it (and I actually think this scheme makes sense), then perhaps near > make_patch_name in stgit/utils.py. > I removed it. Easier to use the standard scheme. > > if options.src: > > iw = None # can't use index/workdir to manipulate another branch > > else: > > iw = src.repository.default_iw > > I would have named this src_iw or something, to make it clear which > branch it belongs to. > Fixed. Also, changed one use of src_iw; we must use None when pushing in order to prevent the conflict from touching the working tree (perhaps this is the error in delete as well?) > > out.info('Created %s on branch %s' % (", ".join(created), dest.name)) > > This line is often going to get very long Fixed, along with silencing delete (also had to fix a bug there, see patch). It now outputs: Successfully moved patches: master:a -> topic:a-0 master:foo -> topic:foo which brings me to a wild class of ideas on argument syntax: We could allow some colon-based notation, yielding a syntax familiar from cross-device mv on systems such as Amiga: stg move df0:foo df1: to move patch foo from branch df0 to branch df1; stg move df0:foo df1:bar to also rename the patch to bar; stg move df0:foo master:bar df1: to move two patches from different branches to branch df1 The latter two cannot be expressed with the current parameter syntax. However, the new uses are rather exotic, and the new syntax does not provide a natural way to move a remote patch to the current branch without naming the current branch. > Also, you don't handle (or test!) the case where -f explicitly > specifies the current branch. Fixed now (kind of): I test that giving the current branch is equivalent to omitting it, in the test where I check that source and destination cannot be the same.
_______________________________________________ stgit-users mailing list [email protected] https://mail.gna.org/listinfo/stgit-users
