Re: [PATCH v5 04/15] git-p4: change the expansion test from basestring to list

From: Ben Keene <seraphire@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Ben Keene via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 04/15] git-p4: change the expansion test from basestring to list
Date: Fri, 13 Dec 2019 09:40:00 -0500	[thread overview]
Message-ID: <7141d2c7-8b6f-d4a9-f6cf-6660f42fb5e1@gmail.com> (raw)
In-Reply-To: <xmqq8snlutyx.fsf@gitster-ct.c.googlers.com>


On 12/9/2019 3:25 PM, Junio C Hamano wrote:
> "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> The original code used 'basestring' in a test to determine if a list or
>> literal string was passed into 9 different functions.  This is used to
>> This is mostly true, but the use in p4_build_cmd() and the second
...
> use among the two uses in p4CmdList() are different.
>
> 	Some codepaths can represent a command line the program
> 	internally prepares to execute either as a single string
> 	(i.e. each token properly quoted, concatenated with $IFS) or
> 	as a list of argv[] elements, and there are 9 places where
> 	we say "if X is isinstance(_, basestring), then do this
> 	thing to handle X as a command line in a single string; if
> 	not, X is a command line in a list form".
>
> 	This does not work well with Python 3, as there is no
> 	basestring (everything is Unicode now), and even with Python
> 	2, it was not an ideal way to tell the two cases apart,
> 	because an internally formed command line could have been in
> 	a single Unicode string.
>
> 	Flip the check to say "if X is not a list, then handle X as
> 	a command line in a single string; otherwise treat it as a
> 	command line in a list form".
>
> 	This will get rid of references to 'basestring', to migrate
> 	the code ready for Python 3.
>
> or something like that?
>
I incorporated this into my suggested submission to Yang's work. Thank you.