[PATCH] automv: new extension

[Matt Mackall]

On Tue, 2016-02-02 at 15:32 +0000, Martijn Pieters wrote:
> # HG changeset patch
> # User mj at zopatista.com
> # Date 1454422084 0
> #Â Â Â Â Â Â Tue Feb 02 14:08:04 2016 +0000
> # Node ID 3bf26557bd41c554e5cf1d2040f5fd0e9062f8e1
> # Parent  e0b374379f4bcc23879d29065fd44457d616a1af
> automv: new extension

This shows up in the release notes, so you'll want to stick experimental too.

> Automatically detect moves and record them

..at commit time

> This extension was originally developed at
> https://bitbucket.org/facebook/hg-experimental
> 
> diff --git a/hgext/automv.py b/hgext/automv.py
> new file mode 100644
> --- /dev/null
> +++ b/hgext/automv.py
> @@ -0,0 +1,101 @@
> +# automv.py
> +#
> +# Copyright 2013 Facebook, Inc.

I know for a fact you haven't been sitting on this for 3 years.

> +#
> +# This software may be used and distributed according to the terms of the
> +# GNU General Public License version 2 or any later version.
> +"""Check for unrecorded moves at at commit time (EXPERIMENTAL)

Double "at".

> +This extension checks at commit/amend time if any of the committed files
> +comes from an unrecorded mv.
> +
> +The threshold at which a file is considered a move can be set with the
> +``automv.similaritythres`` config option; the default value is 0.75.

That's a pretty unfortunate abbreviation, truncating the word right in the
middle of a phoneme. Better to stop at "similarity" if there's some sort of
letter famine on.

Is there a precedent for the 0.75?

> +"""
> +
> +from mercurial import (
> +Â Â Â Â commands,
> +Â Â Â Â copies,
> +Â Â Â Â extensions,
> +Â Â Â Â scmutil,
> +Â Â Â Â similar,
> +)
> +from mercurial.extensions import wrapcommand
> +from mercurial.i18n import _

"_" is one of the few symbol-level imports we like, because of its ubiquity.
Others should be avoided.

> +
> +

Our codebase heartily rejects the PEP8 double newline convention. I believe
check-commit should complain about it.

> +def extsetup(ui):
> +Â Â Â Â entry = wrapcommand(commands.table, 'commit', mvcheck)
> +Â Â Â Â entry[1].append(
> +Â Â Â Â Â Â Â Â ('', 'no-move-detection', None,
> +Â Â Â Â Â Â Â Â Â _('disable automatic file move detection')))

I'd lean towards --no-automv as a hint to where this option comes from. 

> +Â Â Â Â try:
> +Â Â Â Â Â Â Â Â module = extensions.find('fbamend')

It's kinda iffy to have outside references in the main codebase. We can't
effectively maintain their correctness. This particular code is probably broken
anyway, as it depends on module load order. So a) it should probably move to
fbamend and b) use extensions.afterloaded.

> +Â Â Â Â if n == 1:
> +Â Â Â Â Â Â Â Â repo.ui.status(_('detected move of 1 file\n'))
> +Â Â Â Â elif n > 1:
> +Â Â Â Â Â Â Â Â repo.ui.status(_('detected move of %d files\n') % len(renames))

We don't do this either. One day, in the distant future, we'll have an AI that
knows how to fully and correctly localize plurals in all languages. Until then,
"%d files" always please. In this particular case, since we've already reported
the renames at the same verbosity level, best to say nothing.

> Â  Â # developer config: automv.testmode
> +Â Â Â Â if ui.configbool('automv', 'testmode'):
> +Â Â Â Â Â Â Â Â return

This pattern is not really in accord with our test philosophy, which is
"actually do the thing and check what happens".

--Â 
Mathematics is the supreme nostalgia of our time.