Tue, 20 Jun 2017 23:22:38 -0700 patch: rewrite reversehunks (issue5337)
Jun Wu <quark@fb.com> [Tue, 20 Jun 2017 23:22:38 -0700] rev 32997
patch: rewrite reversehunks (issue5337) The old reversehunks code accesses "crecord.uihunk._hunk", which is the raw recordhunk without crecord selection information, therefore "revert -i" cannot revert individual lines, aka. issue5337. The patch rewrites related logic to return the right reverse hunk for revert. Namely, 1. "fromline" and "toline" are correctly swapped [1] 2. crecord.uihunk generates a correct reverse hunk [2] Besides, reversehunks(hunks) will no longer modify its input "hunks", which is more expected. [1]: To explain why "fromline" and "toline" need to be swapped, take the following example: $ cat > a <<EOF > 1 > 2 > 3 > 4 > EOF $ cat > b <<EOF > 2 > 3 > 5 > EOF $ diff a b 1d0 <---- "1" is "fromline" and "0" is "toline" < 1 and they are swapped if diff from the reversed direction 4c3 | < 4 | --- | > 5 | | $ diff b a | 0a1 <---------+ > 1 3c4 <---- also "4c3" gets swapped to "3c4" < 5 --- > 4 [2]: This is a bit tricky. For example, given a file which is empty in working parent but has 3 lines in working copy, and the user selection: select hunk to discard [x] +1 [ ] +2 [x] +3 The user intent is to drop "1" and "3" in working copy but keep "2", so the reverse patch would be something like: -1 2 (2 is a "context line") -3 We cannot just take all selected lines and swap "-" and "+", which will be: -1 -3 That patch won't apply because of "2". So the correct way is to insert "2" as a "context line" by inserting it first then deleting it: -2 +2 Therefore, the correct revert patch is: -1 -2 +2 -3 It could be reordered to look more like a common diff hunk: -1 -2 -3 +2 Note: It's possible to return multiple hunks so there won't be lines like "-2", "+2". But the current implementation is much simpler. For deletions, like the working parent has "1\n2\n3\n" and it was changed to empty in working copy: select hunk to discard [x] -1 [ ] -2 [x] -3 The user intent is to drop the deletion of 1 and 3 (in other words, keep those lines), but still delete "2". The reverse patch is meant to be applied to working copy which is empty. So the patch would be: +1 +3 That is to say, there is no need to special handle the unselected "2" like the above insertion case.
Wed, 21 Jun 2017 10:46:18 +0200 profiling: cope with configwith default value handling changes
Pierre-Yves David <pierre-yves.david@octobus.net> [Wed, 21 Jun 2017 10:46:18 +0200] rev 32996
profiling: cope with configwith default value handling changes Changeset 6ff6eb33f353 change 'configwith' behavior so that the default value is run through the conversion function. In parallel a new user of 'configwith' got introduced unaware of this coming behavior change. This broke profiling. We resolve the situation by having the new conversion function cope with a default value already using the right type.
Tue, 20 Jun 2017 14:00:41 -0700 py3: catch StopIteration from next() in generatorset
Martin von Zweigbergk <martinvonz@google.com> [Tue, 20 Jun 2017 14:00:41 -0700] rev 32995
py3: catch StopIteration from next() in generatorset IIUC, letting the StopIteration through would not cause any bugs, but not doing it makes the test-py3-commands.t pass. I have also diligently gone through all uses of next() in our code base. They either: * are not called from a generator * pass a default value to next() * catch StopException * work on infinite iterators * request a fixed number of items that matches the generated number * are about batching in wireproto which I didn't quite follow I'd appreciate if Augie or someone else could take a look at the wireproto batching and convince themselves that the next(batchable) calls there will not raise a StopIteration.
(0) -30000 -10000 -3000 -1000 -300 -100 -30 -10 -3 +3 +10 +30 +100 +300 +1000 +3000 +10000 tip