Re: [PATCH 4/6] git-p4: Decode byte strings before printing

[Junio C Hamano <gitster@pobox.com>]

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 latter, such a change should be separated into its own step,
or at least needs to be mentioned and justified in the proposed log
message.

Lack of explanation on "why" is shared among all these patches, it
seems, so I won't repeat, but the patches need to explain why to
their readers.

> 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