Previously we'd request all results, then filter by relative root. This is
clearly inefficient, so we now restrict the matcher to the relative root for
certain easy cases.
The particular case here is when the matcher matches all files. In that case we
can simply create a matcher by the relative root.
This is purely an optimization and has no impact on correctness.
For now this implementation is pretty naive -- it filters out files right
before passing them into trydiff. In upcoming patches we'll add some more
smarts.
This has several advantages compared to resolving it relative to the root:
- '--prefix .' works as expected.
- consistent with upcoming 'hg diff' option to produce relative patches
(I made sure to put in the (glob) annotations this time!)
Although Python supports `X = Y if COND else Z`, this was only
introduced in Python 2.5. Since we have to support Python 2.4, it was
a very common thing to write instead `X = COND and Y or Z`, which is a
bit obscure at a glance. It requires some intricate knowledge of
Python to understand how to parse these one-liners.
We change instead all of these one-liners to 4-liners. This was
executed with the following perlism:
find -name "*.py" -exec perl -pi -e 's,(\s*)([\.\w]+) = \(?(\S+)\s+and\s+(\S*)\)?\s+or\s+(\S*)$,$1if $3:\n$1 $2 = $4\n$1else:\n$1 $2 = $5,' {} \;
I tweaked the following cases from the automatic Perl output:
prev = (parents and parents[0]) or nullid
port = (use_ssl and 443 or 80)
cwd = (pats and repo.getcwd()) or ''
rename = fctx and webutil.renamelink(fctx) or []
ctx = fctx and fctx or ctx
self.base = (mapfile and os.path.dirname(mapfile)) or ''
I also added some newlines wherever they seemd appropriate for readability
There are probably a few ersatz ternary operators still in the code
somewhere, lurking away from the power of a simple regex.
Rev cb4d72125aae adding a parameter here. This breaks third-party extensions
like crecord and also makes the issue fairly hard to fix on the extension's
side if it wants to retain compatibility across Mercurial versions -- in old
versions, the positional argument will be passed into the next unknown
argument, which is 'files'.
The patch also undoes a change to the record extension that is no longer
necessary.
This is preparation for upcoming patches that will add support for applying a
patch within a subdirectory.
We normalize the prefix here because this is the main driver -- all code to
apply patches is expected to go through here.
This is preparation for upcoming patches that will add support for applying a
patch within a subdirectory.
The prefix is applied after path components are stripped.
The code that identifies copies/renames, as well as the filenames
before and after, is now isolated and we can extract it to a function
so it can be overridden by extensions (in particular the narrow clone
extension).
There is not much left of the first block "if opts.git or losedatafn"
block now. The next patch will move the call to getfilectx() out of
that block. We will then be using the defined-ness of 'f1' to tell
whether the file existed in ctx1 (and under what name). We will need
this information whether or not opts.git or losedatafn was set, so
just remove that guard. The only operation in the block that is not
cheap is the call to getfilectx(), but that has an extra 'if opts.git'
guard already.
--ignore-space-change proves that only 'if opts.git or losedatafn:'
was removed.
f1 and f2 are currently set always set to some filename, even for
added or deleted files. Let's instead set them to None to indicate
that one side of the diff doesn't exist. This lets us use the filename
variables instead of the content variables and simplify a bit since
the empty string is not a valid filename. More importantly, it paves
the way for further simplifications.
By having all the checks for lossiness in one place, it becomes much
easier to get an overview of the conditions that lead to losedatafn()
being called. It also makes it obvious that it can not be called
multiple times for a single time (something that was rather tricky to
determine before).
It's not obvious, but every path in the 'if opts.git or losedatafn:'
block will have checked whether the file is binary [1]. Let's assign
the result of this check to a variable so we can simplify by checking
'binary and opts.git' in only one place instead of every place we
currently assign to 'binarydiff'.
[1] Except when deleting an empty file, but checking whether an empty
string is binary is very cheap anyway.
It seems natural that each element in the list corresponds to one line
of output. That is currently true, but only because each element in
the list has a trailing newline. Let's drop those newlines and instead
add them when we print the headers.
By moving the condition out of diffline(), the call site becomes
clearer and diffline() no longer closes on any variables.
Note that this changes the value of the header variable from [''] to
[], but there is no difference in how these two are treated by the
following code. The new value seems more natural anyway.
Now that there is only a single call to addmodehdr() left, and there
is other similar code (for new/deleted files) around that call site,
let's inline the function there. That also makes it clearer under what
circumstances the header is actually written (when modes differ).
This is the first step towards simplifying the big loop in
trydiff(). This will make both the header code and the non-header code
clearer, and it prepares for further simplification of the many nested
if-statements in the body of the loop.
Use '1' and '2' as suffix for names just like in the parameters
'ctx[12]':
to,tn -> content1,content2
a,b -> f1, f2
omode,mode -> mode1,mode2
omode,nmode -> mode1,mode2
onode,nnode -> node1,node2
oflag,nflag -> flag1,flag2
oindex,nindex -> index1,index2
When creating a diff with copy/rename enabled, we consider added files
and check if they are either copy sources or targets. However, an
added file should never be a copy source. The test suite seems to
agree with this: all tests pass if we raise an exception when an added
file is a copy source. So, let's simplify the code by dropping the
conditions that are never true.
For those interested in the historical reasons:
Before commit c15c00e7afba (patch: separate reverse copy data
(issue1959), 2010-02-11), 'copy' seems to have been a bidirectional
map. Then that commit split it up into two unidirectional maps and
duplicated the logic to look in both maps. It was still needed at that
point to look in both maps, as the copy detection was poor and could
sometimes be reported in reverse.
A little later came 5a644704d5eb (copies: rewrite copy detection for
non-merge users, 2012-01-04). That commit fixed the copy detection to
be backwards when it should, and made the hacks in trydiff
unnecessary.
We started updating 'modifiedset' in e85e48ffb83e (trydiff: simplify
checking for additions, 2014-12-23) but in the same commit, we removed
the last use of the variable. Clean it up.
