From: Fabian Stelzer <fs@gigacodes.de>
Date: Fri, 10 Dec 2021 08:40:21 GMT
Subject: Re: [PATCH 4/6] git-p4: Decode byte strings before printing
Message-ID: <20211210084021.k4pzkckrmocoqfgg@fs>
In-Reply-To: <xmqqczm5o0pa.fsf@gitster.g>

On 09.12.2021 14:47, Junio C Hamano wrote:
>Joel Holdsworth <jholdsworth@nvidia.com> writes:
>
>> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
>> ---
>>  git-p4.py | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>
>Is the use of strings with {} placeholders and their .format() method
>integral part of "decoding byte strings before printing", or it is just
>a new/better/improved/subjectively-preferred/whatever style?
>

If the new minimum python version will be 3.6 or above I'd vote for using 
f-Strings instead of .format() which I think are more readable and are also 
supposed to be faster.

So:
sys.stdout.write(f'\r{file_path} --> {rel_path} ({size/1024/1024} MB)\n')

instead of one of these:
sys.stdout.write('\r%s --> %s (%i MB)\n' % (file_path, relPath, size/1024/1024))
sys.stdout.write('\r{} --> {} ({} MB)\n'.format(file_path.decode(), relPath, 
size/1024/1024))

>> diff --git a/git-p4.py b/git-p4.py
>> index b5d4fc1176..b5945a0306 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2917,7 +2917,8 @@ def streamOneP4File(self, file, contents):
>>                  size = int(self.stream_file['fileSize'])
>>              else:
>>                  size = 0 # deleted files don't get a fileSize apparently
>> -            sys.stdout.write('\r%s --> %s (%i MB)\n' % (file_path, relPath, size/1024/1024))
>> +            sys.stdout.write('\r{} --> {} ({} MB)\n'.format(
>> +                file_path.decode(), relPath, size/1024/1024))
>>              sys.stdout.flush()
>>
>>          (type_base, type_mods) = split_p4_type(file["type"])
>> @@ -3061,7 +3062,8 @@ def streamP4FilesCb(self, marshalled):
>>              size = int(self.stream_file["fileSize"])
>>              if size > 0:
>>                  progress = 100*self.stream_file['streamContentSize']/size
>> -                sys.stdout.write('\r%s %d%% (%i MB)' % (self.stream_file['depotFile'], progress, int(size/1024/1024)))
>> +                sys.stdout.write('\r{} {}% ({} MB)'.format(
>> +                    self.stream_file['depotFile'].decode(), progress, int(size/1024/1024)))
>>                  sys.stdout.flush()
>>
>>          self.stream_have_file_info = True
>> @@ -3803,7 +3805,7 @@ def closeStreams(self):
>>              return
>>          self.gitStream.close()
>>          if self.importProcess.wait() != 0:
>> -            die("fast-import failed: %s" % self.gitError.read())
>> +            die("fast-import failed: {}".format(self.gitError.read().decode()))
>>          self.gitOutput.close()
>>          self.gitError.close()
>>          self.gitStream = None