Re: [PATCH 1/5] difftool: Simplify print_tool_help()

2012-07-24 Thread Tim Henigan
On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote:
 Eliminate a global variable and File::Find usage by building upon
 basename() and glob() instead.

glob was used in an early revision of the patch that led to bf73fc2
(difftool: print list of valid tools with '--tool-help') as well [1].
However, if the path to git or the path under 'mergetools' includes
spaces, glob fails.  To work around the problem, File::Find was used
instead [2].

Does this implementation handle that case?  I'm sorry, but I haven't
had time to apply and test myself.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925
[2]: http://thread.gmane.org/gmane.comp.version-control.git/194158
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] difftool: Simplify print_tool_help()

2012-07-24 Thread David Aguilar
On Tue, Jul 24, 2012 at 5:43 AM, Tim Henigan tim.heni...@gmail.com wrote:
 On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote:
 Eliminate a global variable and File::Find usage by building upon
 basename() and glob() instead.

 glob was used in an early revision of the patch that led to bf73fc2
 (difftool: print list of valid tools with '--tool-help') as well [1].
 However, if the path to git or the path under 'mergetools' includes
 spaces, glob fails.  To work around the problem, File::Find was used
 instead [2].

 Does this implementation handle that case?  I'm sorry, but I haven't
 had time to apply and test myself.

 [1]: 
 http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925
 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158

Good catch.  Eliminating the globals should be the goal, then.
I'll have to re-roll.
-- 
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] difftool: Simplify print_tool_help()

2012-07-24 Thread David Aguilar
On Tue, Jul 24, 2012 at 6:52 PM, David Aguilar dav...@gmail.com wrote:
 On Tue, Jul 24, 2012 at 5:43 AM, Tim Henigan tim.heni...@gmail.com wrote:
 On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote:
 Eliminate a global variable and File::Find usage by building upon
 basename() and glob() instead.

 glob was used in an early revision of the patch that led to bf73fc2
 (difftool: print list of valid tools with '--tool-help') as well [1].
 However, if the path to git or the path under 'mergetools' includes
 spaces, glob fails.  To work around the problem, File::Find was used
 instead [2].

 Does this implementation handle that case?  I'm sorry, but I haven't
 had time to apply and test myself.

 [1]: 
 http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925
 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158

 Good catch.  Eliminating the globals should be the goal, then.
 I'll have to re-roll.

These landed in next.

Junio, would you prefer follow-up patches or a rebased series?
-- 
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] difftool: Simplify print_tool_help()

2012-07-24 Thread Junio C Hamano
David Aguilar dav...@gmail.com writes:

 Does this implementation handle that case?  I'm sorry, but I haven't
 had time to apply and test myself.

 [1]: 
 http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925
 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158

 Good catch.  Eliminating the globals should be the goal, then.
 I'll have to re-roll.

 Junio, would you prefer follow-up patches or a rebased series?

Incremental patches, please.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] difftool: Simplify print_tool_help()

2012-07-24 Thread David Aguilar
On Tue, Jul 24, 2012 at 7:40 PM, Junio C Hamano gits...@pobox.com wrote:
 David Aguilar dav...@gmail.com writes:

 Does this implementation handle that case?  I'm sorry, but I haven't
 had time to apply and test myself.

 [1]: 
 http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925
 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158

 Good catch.  Eliminating the globals should be the goal, then.
 I'll have to re-roll.

 Junio, would you prefer follow-up patches or a rebased series?

 Incremental patches, please.

 Thanks.

Okay, I saw this just before hitting send-email (and sent the whole series).

I'll prep the incremental ones shortly, so please ignore the one I just sent...
-- 
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html