| Submitter | Matt Harbison |
|---|---|
| Date | Nov. 2, 2014, 8:58 p.m. |
| Message ID | <f72827d6b96cb953432f.1414961901@Envy> |
| Download | mbox | patch |
| Permalink | /patch/6533/ |
| State | Accepted |
| Headers | show |
Comments
On 11/02/2014 08:58 PM, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1414958330 18000 > # Sun Nov 02 14:58:50 2014 -0500 > # Branch stable > # Node ID f72827d6b96cb953432fff89a0bd3b13a142e826 > # Parent cc1cbb0bba8ed1d95c8f1b8e27d4d2893e0dcca7 > filemerge: split the logic for finding an external tool to its own function > > This will be used by extdiff in an subsequent patch. > > diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py > --- a/mercurial/filemerge.py > +++ b/mercurial/filemerge.py > @@ -37,6 +37,9 @@ > def _findtool(ui, tool): > if tool in internals: > return tool > + return findexternaltool(ui, tool) > + > +def findexternaltool(ui, tool): > for kn in ("regkey", "regkeyalt"): > k = _toolstr(ui, tool, kn) > if not k: The docstring deiti would be pleased to see one. And you will be happy to not see this function being dropped in 6 month because it had no obvious other caller and no explanation of its purpose in life ;-) I like where this series is going otherwise.
Pierre-Yves David wrote: > > > On 11/02/2014 08:58 PM, Matt Harbison wrote: >> # HG changeset patch >> # User Matt Harbison <matt_harbison@yahoo.com> >> # Date 1414958330 18000 >> # Sun Nov 02 14:58:50 2014 -0500 >> # Branch stable >> # Node ID f72827d6b96cb953432fff89a0bd3b13a142e826 >> # Parent cc1cbb0bba8ed1d95c8f1b8e27d4d2893e0dcca7 >> filemerge: split the logic for finding an external tool to its own >> function >> >> This will be used by extdiff in an subsequent patch. >> >> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py >> --- a/mercurial/filemerge.py >> +++ b/mercurial/filemerge.py >> @@ -37,6 +37,9 @@ >> def _findtool(ui, tool): >> if tool in internals: >> return tool >> + return findexternaltool(ui, tool) >> + >> +def findexternaltool(ui, tool): >> for kn in ("regkey", "regkeyalt"): >> k = _toolstr(ui, tool, kn) >> if not k: > > The docstring deiti would be pleased to see one. And you will be happy > to not see this function being dropped in 6 month because it had no > obvious other caller and no explanation of its purpose in life ;-) > > I like where this series is going otherwise. > > OK, easy enough. I thought about it, but didn't because it is doing less than the undocumented function it was split from. Is that the secondary purpose of docstring- i.e. indicate that it might be used elsewhere? I can't recall seeing one that explicitly said it was used elsewhere. I'll wait a day and see if anyone else has feedback before resending. --Matt
Patch
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py --- a/mercurial/filemerge.py +++ b/mercurial/filemerge.py @@ -37,6 +37,9 @@ def _findtool(ui, tool): if tool in internals: return tool + return findexternaltool(ui, tool) + +def findexternaltool(ui, tool): for kn in ("regkey", "regkeyalt"): k = _toolstr(ui, tool, kn) if not k: