1. Oleg Oshmyan
  2. hgshelve
  3. Pull requests

Pull requests

#10 Open
Repository
chwidmer
Branch
default
Repository
astiob
Branch
default

Fixes and Extentions

Bitbucket cannot automatically merge this request.

The commits that make up this pull request have been removed.

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

hg update default
hg pull -r default https://bitbucket.org/chwidmer/hgshelve
hg merge 0a76ee65495c
hg commit -m 'Merged in chwidmer/hgshelve (pull request #10)'
Author
  1. Christian Widmer
Reviewers
Description

The change log entries should contain all required information. If not let me know.

Comments (3)

  1. Oleg Oshmyan repo owner

    First of all, will you mind me editing your commits to correct minor things like typos and trailing whitespace?

    I’ll add inline comments to some of your commits in a moment.

    In general you seem to be trying to rewrite too much code. Now, commit by commit:

    • Fix range information patching

      Good. Just one observation: the line numbers in the non-shelved hunks are also wrong, but they probably don’t matter because they are tolines rather than fromlines. (I say that, but clearly the record extension does correct toline numbers, so maybe there is a good reason to do so that I just can’t think of.)

    • Fix appending patches

      Good.

    • Sort shelves alphabetically

      Good. Not only useful for tests but also more user-friendly.

    • Add support for ‘added’ and ‘removed’ files

      The new functionality plays badly with untracked files. Given this:

      echo Lorem ipsum. > a
      hg add a
      hg commit a -m 'Added a'
      

      consider:

      hg forget a # or hg rm a
      echo Some very important data. > a
      hg shelve --all
      

      or

      hg forget a # or hg rm a
      hg shelve --all
      echo Some very important data. > a
      

      or

      echo Some very important data. > a
      hg forget a
      hg shelve --all
      

      In all three cases, some very important data is lost. Also, less importantly, in this scenario:

      hg forget a
      hg shelve --all
      ^C
      

      shelving fails, and in an attempt to restore the original repository state, hgshelve deletes a from the working directory. This doesn’t cause data loss because a can be checked out from the parent revision, but it probably does cause inconvenience for the user.

    I’ve edited these four commits and pushed them to my personal fork. Please take a look, and if you’re happy with my changes, I’m willing to pull them into the official repository.

    • Add keep option to shelve and unshelve

      What benefits does this have over hg diff > my.patch?

      Is appending handled correctly? Shouldn’t it actually prepend?

    • Add delete option to unshelve

      Good (except see the inline comments).

    • Add test for ‘keep’ and ‘delete’

      Good.

    • Add support to split/edit a patch while shelving

      Most of this commit seems to be basic editing of existing code: renaming variables, splitting lines, moving function definitions, removing uses of the tuple parameter syntax, replacing loops with list displays and generator expressions, replacing 'y' and 'n' with True and False. Is this done for consistency with the record extension? (If so, good.) In any case, there are so many of these edits that it is hard to distinguish them from actual new functionality. They should be split out into a changeset of their own.

      By the way, you include the header.rename function but not the rename_re constant that it references. Either remove the function or add the constant.

      Some lines are longer than 80 characters and should be split or replaced with function calls.

    • Add support to shelve ‘renamed’ files

      Looks mostly good, but I haven’t tested it. The last hgshelve.py hunk is not needed if you’re a bit more careful back in the ‘Add support for “added” and “removed” files’ changeset.

    • Add support to let hunk undecided at first

      So this is a feature that the record extension doesn’t have.

      • Probably remove class renames, particularly since the record extension keeps them in lowercase.
      • I haven’t tested this, so I’ll ask: how do hunk indices change between loop iterations?
      • Remove trailing whitespace.
    • Added tag 0.9.0.0rc1 for changeset 5ffe7d3bf495

      Is there any need for this? And why such a strange version number? And in any case, there should be only one line in .hgtags but you have two.

    • Fix regression of rename introduced by 5ffe7d3

      Integrate the fix into the original changeset.

  2. Oleg Oshmyan repo owner

    Oh, I should make myself a bit clearer. The changeset that supports added and removed files does play badly with untracked files, and it would be nice to fix this to give a warning and refuse to proceed if an untracked file would be overwritten, but MQ has this problem too, so I’m willing to accept the patch as it is now.