Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decode log messages on windows as UTF-8 #153

Closed
wants to merge 1 commit into from
Closed

Decode log messages on windows as UTF-8 #153

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions svn/common_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@

import os
import platform
import subprocess
import sys
import logging

import svn.config
Expand Down Expand Up @@ -30,6 +33,13 @@ def external_command(
else:
kwargs['stderr'] = subprocess.STDOUT

# This allows correct decoding of commit messages on Windows
# subprocess.Popen defaults to locale.getpreferredencoding()
# which can be cp1252 (for example)
if (decode_text and platform.system() == "Windows"
and sys.version_info >= (3, 6)):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version seems arbitrary. It's not good practice to hard-code specific versions. If absolutely necessary, it's better to test for the features/characteristics that you actually need. In this case, you can just pass universal_newlines=True, which is supported in both 2.7 and 3.x .

Copy link
Owner

@dsoprea dsoprea May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're already passing universal_newlines (

PySvn/svn/common_base.py

Line 39 in e8a482d

universal_newlines=decode_text,
), it will already always be returned as UTF-8 on Python 2.7 and string in Python 3.x in situations where text is expected. This pull-request is either in response to a not-understand problem or it's a symptom of a different problem. In which context are you encountering this?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open an issue with your usage and the error that you're encountering and go from there.

kwargs['encoding'] = 'utf-8'

p = \
subprocess.Popen(
cmd,
Expand Down