Merge lp://staging/~ds3/ubuntu/oneiric/synaptic/fix-for-63974 into lp://staging/ubuntu/oneiric/synaptic

Proposed by Douglas Schneider
Status: Needs review
Proposed branch: lp://staging/~ds3/ubuntu/oneiric/synaptic/fix-for-63974
Merge into: lp://staging/ubuntu/oneiric/synaptic
Diff against target: 71 lines (+26/-4) (has conflicts)
2 files modified
debian/changelog (+10/-0)
gtk/rgmainwindow.cc (+16/-4)
Text conflict in debian/changelog
To merge this branch: bzr merge lp://staging/~ds3/ubuntu/oneiric/synaptic/fix-for-63974
Reviewer Review Type Date Requested Status
Colin Watson (community) Needs Fixing
Review via email: mp+74038@code.staging.launchpad.net

Description of the change

gtk/rgmainwindow.cc was modified to make it so that when a user tries to delete a package that is (likely) essential to the os - bash for example - the warning is now given with the offending packages name. This makes it easy for a user to know which packages they should not be deleting if they try to delete many packages at the same time and some are essential.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

Thanks for your patch.

This will break translations. You can't assemble strings at run-time like this and have the result be translatable (because different languages will need to assemble the string in different orders); what you need to do instead is translate a format string and then substitute the package name into that. Awkwardly, RUserDialog::confirm only takes a single string rather than a format string plus arguments, and that looks like a pain to change, but you can just substitute it manually.

Annoyingly, vanilla C++ lacks a way to deal with format strings, aside from the C sprintf/snprintf functions which require deciding on a maximum size for the translated string (even if you dodge buffer overflow issues). I would suggest '#define _GNU_SOURCE 1' at the top of the file and then you can use asprintf. The result would then look something like this (untested):

  char *warning_fmt = _("Removing the %s package may render the "
                        "system usable.\n"
                        "Are you sure you want to do that?")
  char *warning;

  if (asprintf(&warning, warning_fmt, pkg->name()) < 0) {
     perror("asprintf");
     return;
  }
  bool confirmed = _userDialog->confirm(warning, false);
  free(warning);
  if (!confirmed)
     return;

(Not very C++ish, but hopefully Michael will have a look at this at some point and see if he can suggest anything neater!)

review: Needs Fixing
115. By Douglas Schneider

Updated the warning for deleting a required package to use memory allocation.
(LP: #63974)

Revision history for this message
Douglas Schneider (ds3) wrote :

The translations need to be updated for the new warning message on line 1887 of gtk/rgmainwindow.cc.

Unmerged revisions

115. By Douglas Schneider

Updated the warning for deleting a required package to use memory allocation.
(LP: #63974)

114. By Douglas Schneider

updated the change log

113. By Douglas Schneider

changed the warning message for deleting an important package to supply the packages name

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