command-not-found

Merge lp://qastaging/~xintx-ua/command-not-found/add-install-support into lp://qastaging/~command-not-found-developers/command-not-found/trunk

  1. add-install-support
  2. Merge into trunk
Proposed by Serhiy
Status: Merged
Merged at revision: 144
Proposed branch: lp://qastaging/~xintx-ua/command-not-found/add-install-support
Merge into: lp://qastaging/~command-not-found-developers/command-not-found/trunk
Diff against target: 49 lines (+14/-0)
2 files modified
CommandNotFound/CommandNotFound.py (+13/-0)
command-not-found (+1/-0)
To merge this branch: bzr merge lp://qastaging/~xintx-ua/command-not-found/add-install-support
Reviewer Review Type Date Requested Status
Zygmunt Krynicki Approve
Review via email: mp+84257@code.qastaging.launchpad.net

Description of the change

Adding installation support.

To post a comment you must log in.
Revision history for this message
Zygmunt Krynicki (zyga) wrote :

21 + locale.setlocale(locale.LC_ALL, '')

Why are you setting this?

+ if return_code not in (0,256): #256 = user rejected installation
29 + print >> sys.stderr, _("A problem occured during installation. apt-get returned %d") % return_code

I would just do if return_code != 0, I don't know if there is any value as the failing installation probably leaves enough information to the user anyway.

lp://qastaging/~xintx-ua/command-not-found/add-install-support updated
145. By Serhiy

Fixes localized YESEXPR.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Two more comments:

1) Get the input encoding from locale.getlocale(locale.LC_CTYPE)[1], don't hardcode UTF-8

Come to think of it perhaps that's the same way we should get the input string encoding (not locale.getpreferredencoding(), anyway, it's not something you need to fix in this patch.

2) Run pyflakes and pep8 on this code, it will show you a number of stylistic issues, I can fix that myself but I usually recommend submitters to do it if they can.

lp://qastaging/~xintx-ua/command-not-found/add-install-support updated
146. By Serhiy

Deletes apt-get install exit status handling.

147. By Serhiy

Fixes encoding handling.

148. By Serhiy

Clean-up TODO.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

+1 apart from the locale.YESEXPR that is unusable IMHO. There is no way to display the pattern in a readable manner and there is no way to synchronize local translation to the value returned by the locale system. I'll patch this away and merge.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches