Merge lp:~seb128/apturl/dont-decode-str into lp:apturl

Proposed by Sebastien Bacher
Status: Merged
Merged at revision: 135
Proposed branch: lp:~seb128/apturl/dont-decode-str
Merge into: lp:apturl
Diff against target: 12 lines (+1/-1)
1 file modified
AptUrl/AptUrl.py (+1/-1)
To merge this branch: bzr merge lp:~seb128/apturl/dont-decode-str
Reviewer Review Type Date Requested Status
Steve Langasek Approve
Brian Murray Pending
Michael Vogt Pending
Ubuntu Core Development Team Pending
Review via email: mp+197804@code.launchpad.net

Commit message

don't use decode on a str

Description of the change

don't use decode on a str

To post a comment you must log in.
Revision history for this message
Sebastien Bacher (seb128) wrote :

The issue there is that "decode" is being use on a str type, which doesn't work

looking through the history
r118, Steve added the decode, the variable was a an "unicode" type, http://bazaar.launchpad.net/~ubuntu-core-dev/apturl/ubuntu/revision/118
r120, Brian ported to python3, changing the type to a "str" but keeping the decode, http://bazaar.launchpad.net/~ubuntu-core-dev/apturl/ubuntu/revision/120

The issue is trivial to trigger, just call "apturl-gtk /local/dir/binary"

The change fixes the assertion, not sure if the utf8 handling is an issue with python3 though, or if that's adding back the issue Steve was solving (there is no bug reference on that commit) ... review from people who know how python2/3 handle encodings would be welcome

Revision history for this message
Brian Murray (brian-murray) wrote :

r117 refers to bug 911144.

revno: 117
fixes bug: https://launchpad.net/bugs/911144
committer: Steve Langasek <email address hidden>
branch nick: apturl
timestamp: Sat 2012-03-24 02:23:57 -0700
message:
  Take the unicode representation of the parser error message instead
  of the string representation, so that we get clean i18n handling.
  LP: #911144.

So r118 seems to be a follow up to that.

Revision history for this message
Sebastien Bacher (seb128) wrote :

The testcase from that bug seems to be handled fine with the version here

Revision history for this message
Steve Langasek (vorlon) wrote :

This looks right to me. The url always comes to us from optparse, which should always return a Unicode string in python3 already - so there's no need to decode it.

Note that actual use of unicode in the args to apturl-gtk fails, but that's not the original bug that was reported, and not a likely scenario so we don't really need to fix that here. (Actually, the backtrace in that case appears to be in python-gi/gtk anyway...)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'AptUrl/AptUrl.py'
2--- AptUrl/AptUrl.py 2012-06-14 15:38:19 +0000
3+++ AptUrl/AptUrl.py 2013-12-04 22:48:18 +0000
4@@ -144,7 +144,7 @@
5 self.ui.error(_("Need a url to continue, exiting"))
6 return []
7 except Parser.InvalidUrlException as e:
8- self.ui.error(_("Invalid url: '%s' given, exiting") % e.url.decode('utf-8'),
9+ self.ui.error(_("Invalid url: '%s' given, exiting") % e.url,
10 str(e))
11 return []
12 return (apturl_list)

Subscribers

People subscribed via source and target branches