Merge lp:~andrebachmann-dd/bzr-explorer/fix-bzrlog into lp:bzr-explorer

Proposed by André Bachmann
Status: Merged
Approved by: Martin Packman
Approved revision: 567
Merged at revision: 566
Proposed branch: lp:~andrebachmann-dd/bzr-explorer/fix-bzrlog
Merge into: lp:bzr-explorer
Diff against target: 39 lines (+5/-6)
2 files modified
NEWS (+3/-0)
lib/app_suite.py (+2/-6)
To merge this branch: bzr merge lp:~andrebachmann-dd/bzr-explorer/fix-bzrlog
Reviewer Review Type Date Requested Status
Martin Packman (community) Approve
Review via email: mp+117291@code.launchpad.net

Description of the change

Simple fix for the problem with Bazaar on Windows that you got a Windows error 123 when trying to view the bzr system log via bzr-explorer.

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :

You need to at least update the comments as you're updating the code. :)

Only real question is what version the split function was fixed, against how far back bzr-explorer tries to remain compatible. Checking now, seems it's been in bzrlib since 2.2, and bzr-explorer was updated in r513 to use the unbroken version, but these comments were missed so the quote hack didn't get reverted.

A cleanup of this module not to use command line strings would be nice, but isn't needed here. Just fix or delete the outdated comments and this will do.

review: Needs Fixing
567. By PKO

Removed outdated comments

Revision history for this message
André Bachmann (andrebachmann-dd) wrote :

> You need to at least update the comments as you're updating the code. :)
>
> Only real question is what version the split function was fixed, against how
> far back bzr-explorer tries to remain compatible. Checking now, seems it's
> been in bzrlib since 2.2, and bzr-explorer was updated in r513 to use the
> unbroken version, but these comments were missed so the quote hack didn't get
> reverted.
>
> A cleanup of this module not to use command line strings would be nice, but
> isn't needed here. Just fix or delete the outdated comments and this will do.

I removed the old comments.

Revision history for this message
Martin Packman (gz) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2012-07-12 12:01:41 +0000
3+++ NEWS 2012-08-08 15:05:41 +0000
4@@ -15,6 +15,9 @@
5
6 Bug fixes:
7
8+* Selecting "Bazaar, Explorer, System Log" no longer produces a
9+ Windows Error 123 (André Bachmann, Bug #1021253)
10+
11
12 1.3.0 12-Jul-2012
13 -----------------
14
15=== modified file 'lib/app_suite.py'
16--- lib/app_suite.py 2012-02-20 07:12:07 +0000
17+++ lib/app_suite.py 2012-08-08 15:05:41 +0000
18@@ -170,9 +170,7 @@
19 "plugins": "bzr qplugins",
20 "version": "bzr qversion",
21 "config:bazaar.conf": "bzr qconfig",
22- # Note: We need to quote BZR_LOG here as shlex_split_unicode() is buggy
23- # on Windows
24- ".bzr.log": "bzr qviewer '%(BZR_LOG)s'",
25+ ".bzr.log": "bzr qviewer %(BZR_LOG)s",
26 }
27 _QBZR_LOCAL_MAPPING = {
28 "add": "bzr qadd --ui-mode %(selected)s",
29@@ -226,9 +224,7 @@
30 "checkout": "bzr gcheckout %(filename)s",
31 "init": "bzr ginit",
32 "config:bazaar.conf": "bzr gpreferences",
33- # Note: We need to quote BZR_LOG here as shlex_split_unicode() is buggy
34- # on Windows
35- ".bzr.log": "bzr qviewer '%(BZR_LOG)s'",
36+ ".bzr.log": "bzr qviewer %(BZR_LOG)s",
37 # These aren't supported but ought to be
38 #"version": "bzr gversion",
39 }

Subscribers

People subscribed via source and target branches