BUG 1922588: SDK Updater, Needs better license display

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/AddonPackage.java

@@ -196,9 +196,9 @@ public class AddonPackage extends Package {
     /** Returns a long description for an {@link IDescription}. */
     @Override
     public String getLongDescription() {
-        return String.format("%1$s.\n%2$s",
+        return String.format("%1$s,\nRevision %2$d.",
                 getShortDescription(),
-                super.getLongDescription());
+                getRevision());
     }
 
     /**

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/DocPackage.java

@@ -113,9 +113,9 @@ public class DocPackage extends Package {
     /** Returns a long description for an {@link IDescription}. */
     @Override
     public String getLongDescription() {
-        return String.format("%1$s.\n%2$s",
+        return String.format("%1$s,\nRevision %2$d.",
                 getShortDescription(),
-                super.getLongDescription());
+                getRevision());
     }
 
     /**

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/ExtraPackage.java

@@ -192,8 +192,7 @@ public class ExtraPackage extends Package {
             s += String.format(" (min tools rev.: %1$d)", mMinToolsRevision);
         }
 
-        s += ".\n";
+        s += ".";
-        s += super.getLongDescription();
 
         return s;
     }

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/LocalSdkParser.java

@@ -44,7 +44,11 @@ public class LocalSdkParser {
     }
 
     /**
-     * Returns the packages found by the last call to {@link #parseSdk(String, SdkManager, ISdkLog)}.
+     * Returns the packages found by the last call to
+     * {@link #parseSdk(String, SdkManager, ISdkLog)}.
+     * <p/>
+     * This returns initially returns null.
+     * Once the parseSdk() method has been called, this returns a possibly empty but non-null array.
      */
     public Package[] getPackages() {
         return mPackages;

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/Package.java

@@ -336,8 +336,11 @@ public abstract class Package implements IDescription {
         if (s != null) {
             sb.append(s);
         }
+        if (sb.length() > 0) {
+            sb.append("\n");
+        }
 
-        sb.append(String.format("\nRevision %1$d", getRevision()));
+        sb.append(String.format("Revision %1$d", getRevision()));
 
         s = getDescUrl();
         if (s != null && s.length() > 0) {

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/PlatformPackage.java

@@ -160,9 +160,7 @@ public class PlatformPackage extends Package {
     /** Returns a long description for an {@link IDescription}. */
     @Override
     public String getLongDescription() {
-        return String.format("%1$s.\n%2$s",
+        return getShortDescription() + ".";
-                getShortDescription(),
-                super.getLongDescription());
     }
 
     /**

tools/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/ToolPackage.java

@@ -76,9 +76,7 @@ public class ToolPackage extends Package {
     /** Returns a long description for an {@link IDescription}. */
     @Override
     public String getLongDescription() {
-        return String.format("Android SDK Tools, revision %1$d.\n%2$s",
+        return getShortDescription() + ".";
-                getRevision(),
-                super.getLongDescription());
     }
 
     /**

tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/RemotePackagesPage.java

@@ -117,26 +117,31 @@ public class RemotePackagesPage extends Composite implements ISdkListener {
         mDescriptionLabel.setText("Line1\nLine2\nLine3");
 
         mAddSiteButton = new Button(parent, SWT.NONE);
+        mAddSiteButton.setText("Add Site...");
+        mAddSiteButton.setToolTipText("Allows you to enter a new user external site. " +
+                "Such site can only contribute add-ons and extra packages.");
         mAddSiteButton.addSelectionListener(new SelectionAdapter() {
             @Override
             public void widgetSelected(SelectionEvent e) {
                 onAddSiteSelected(); //$hide$
             }
         });
-        mAddSiteButton.setText("Add Site...");
 
         mDeleteSiteButton = new Button(parent, SWT.NONE);
+        mDeleteSiteButton.setText("Delete Site...");
+        mDeleteSiteButton.setToolTipText("Allows you to remove an external site. " +
+                "Built-in sites cannot be removed.");
         mDeleteSiteButton.addSelectionListener(new SelectionAdapter() {
             @Override
             public void widgetSelected(SelectionEvent e) {
                 onRemoveSiteSelected(); //$hide$
             }
         });
-        mDeleteSiteButton.setText("Delete Site...");
 
         mUpdateOnlyCheckBox = new Button(parent, SWT.CHECK);
         mUpdateOnlyCheckBox.setLayoutData(new GridData(SWT.CENTER, SWT.CENTER, true, false, 1, 1));
         mUpdateOnlyCheckBox.setText("Display updates only");
+        mUpdateOnlyCheckBox.setToolTipText("When selected, only compatible update packages are shown in the list above.");
         mUpdateOnlyCheckBox.setSelection(mUpdaterData.getSettingsController().getShowUpdateOnly());
         mUpdateOnlyCheckBox.addSelectionListener(new SelectionAdapter() {
             @Override
@@ -146,22 +151,25 @@ public class RemotePackagesPage extends Composite implements ISdkListener {
         });
 
         mRefreshButton = new Button(parent, SWT.NONE);
+        mRefreshButton.setText("Refresh");
+        mRefreshButton.setToolTipText("Refreshes the list of packages from open sites.");
         mRefreshButton.addSelectionListener(new SelectionAdapter() {
             @Override
             public void widgetSelected(SelectionEvent e) {
                 onRefreshSelected(); //$hide$
             }
         });
-        mRefreshButton.setText("Refresh");
 
         mInstallSelectedButton = new Button(parent, SWT.NONE);
+        mInstallSelectedButton.setText("Install Selected");
+        mInstallSelectedButton.setToolTipText("Allows you to review all selected packages " +
+                "and install them.");
         mInstallSelectedButton.addSelectionListener(new SelectionAdapter() {
             @Override
             public void widgetSelected(SelectionEvent e) {
                 onInstallSelectedArchives();  //$hide$
             }
         });
-        mInstallSelectedButton.setText("Install Selected");
     }
 
     @Override
@@ -275,7 +283,9 @@ public class RemotePackagesPage extends Composite implements ISdkListener {
     }
 
     private void onShowUpdateOnly() {
-        mUpdaterData.getSettingsController().setShowUpdateOnly(mUpdateOnlyCheckBox.getSelection());
+        SettingsController controller = mUpdaterData.getSettingsController();
+        controller.setShowUpdateOnly(mUpdateOnlyCheckBox.getSelection());
+        controller.saveSettings();
         mTreeViewerSources.refresh();
     }
 
@@ -405,6 +415,9 @@ public class RemotePackagesPage extends Composite implements ISdkListener {
         mDeleteSiteButton.setEnabled(hasSelectedUserSource);
         mRefreshButton.setEnabled(true);
         mInstallSelectedButton.setEnabled(hasCheckedArchive);
+
+        // set value on the show only update checkbox
+        mUpdateOnlyCheckBox.setSelection(mUpdaterData.getSettingsController().getShowUpdateOnly());
     }
 
     // End of hiding from SWT Designer

tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdateChooserDialog.java

@@ -28,6 +28,8 @@ import org.eclipse.jface.viewers.TableViewer;
 import org.eclipse.jface.viewers.Viewer;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.SashForm;
+import org.eclipse.swt.custom.StyleRange;
+import org.eclipse.swt.custom.StyledText;
 import org.eclipse.swt.events.ControlAdapter;
 import org.eclipse.swt.events.ControlEvent;
 import org.eclipse.swt.events.SelectionAdapter;
@@ -48,7 +50,6 @@ import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.swt.widgets.Table;
 import org.eclipse.swt.widgets.TableColumn;
-import org.eclipse.swt.widgets.Text;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -88,7 +89,7 @@ final class UpdateChooserDialog extends Dialog {
     private TableViewer mTableViewPackage;
     private Table mTablePackage;
     private TableColumn mTableColum;
-    private Text mPackageText;
+    private StyledText mPackageText;
     private Button mLicenseRadioAccept;
     private Button mLicenseRadioReject;
     private Button mLicenseRadioAcceptAll;
@@ -99,11 +100,15 @@ final class UpdateChooserDialog extends Dialog {
 
     /**
      * Create the dialog.
+     * @param parentShell The shell to use, typically updaterData.getWindowShell()
      * @param updaterData The updater data
      * @param newToOldUpdates The map [new archive => old archive] of potential updates
      */
-    public UpdateChooserDialog(UpdaterData updaterData, Map<Archive, Archive> newToOldUpdates) {
+    public UpdateChooserDialog(Shell parentShell,
-        super(updaterData.getWindowShell(), SWT.APPLICATION_MODAL);
+            UpdaterData updaterData,
+            Map<Archive, Archive> newToOldUpdates) {
+        super(parentShell,
+              SWT.APPLICATION_MODAL);
         mUpdaterData = updaterData;
 
         mNewToOldArchiveMap = new TreeMap<Archive, Archive>(new Comparator<Archive>() {
@@ -207,13 +212,13 @@ final class UpdateChooserDialog extends Dialog {
         mPackageTextGroup.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true, 4, 1));
         mPackageTextGroup.setLayout(new GridLayout(1, false/*makeColumnsEqual*/));
 
-        mPackageText = new Text(mPackageTextGroup,
+        mPackageText = new StyledText(mPackageTextGroup,
-                SWT.MULTI | SWT.READ_ONLY | SWT.WRAP | SWT.V_SCROLL);
+                        SWT.MULTI | SWT.READ_ONLY | SWT.WRAP | SWT.V_SCROLL);
+        mPackageText.setBackground(
                 getParent().getDisplay().getSystemColor(SWT.COLOR_WIDGET_BACKGROUND));
         mPackageText.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true, 1, 1));
 
         mLicenseRadioAccept = new Button(mPackageRootComposite, SWT.RADIO);
-        mLicenseRadioAccept.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, false, false, 1, 1));
         mLicenseRadioAccept.setText("Accept");
         mLicenseRadioAccept.addSelectionListener(new SelectionAdapter() {
             @Override
@@ -223,7 +228,6 @@ final class UpdateChooserDialog extends Dialog {
         });
 
         mLicenseRadioReject = new Button(mPackageRootComposite, SWT.RADIO);
-        mLicenseRadioReject.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, false, false, 1, 1));
         mLicenseRadioReject.setText("Reject");
         mLicenseRadioReject.addSelectionListener(new SelectionAdapter() {
             @Override
@@ -236,8 +240,6 @@ final class UpdateChooserDialog extends Dialog {
         placeholder.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, true, false, 1, 1));
 
         mLicenseRadioAcceptAll = new Button(mPackageRootComposite, SWT.RADIO);
-        mLicenseRadioAcceptAll.setLayoutData(
-                new GridData(SWT.LEFT, SWT.CENTER, false, false, 1, 1));
         mLicenseRadioAcceptAll.setText("Accept All");
         mLicenseRadioAcceptAll.addSelectionListener(new SelectionAdapter() {
             @Override
@@ -317,7 +319,6 @@ final class UpdateChooserDialog extends Dialog {
         mTablePackage.select(0);
         onPackageSelected();
     }
-
 
     /**
      * Creates the icon of the window shell.
@@ -445,27 +446,46 @@ final class UpdateChooserDialog extends Dialog {
             mPackageText.setText("Please select a package.");
             return;
         }
-        StringBuilder sb = new StringBuilder();
+
+        mPackageText.setText("");                                               //$NON-NLS-1$
+
+        addSectionTitle("Package Description\n");
         addText(a.getParentPackage().getLongDescription(), "\n\n");             //$NON-NLS-1$
 
         Archive aold = mNewToOldArchiveMap.get(a);
-            sb.append("*** Existing Package Description:\n");
+        if (aold != null) {
-            sb.append(aold.getParentPackage().getLongDescription()).append("\n\n");
+            addText(String.format("This update will replace revision %1$s with revision %2$s.\n\n",
+                    aold.getParentPackage().getRevision(),
                     a.getParentPackage().getRevision()));
         }
-        sb.append("*** New Package Description:\n");
-        sb.append(a.getParentPackage().getLongDescription()).append("\n\n");
 
-        sb.append("\n*** Archive Description:\n");
+
-        sb.append(a.getLongDescription()).append("\n");
+        addSectionTitle("Archive Description\n");
         addText(a.getLongDescription(), "\n\n");                                //$NON-NLS-1$
 
         String license = a.getParentPackage().getLicense();
-            sb.append("\n*** Package License:\n");
+        if (license != null) {
-            sb.append(license).append("\n");
+            addSectionTitle("License\n");
             addText(license.trim(), "\n");                                      //$NON-NLS-1$
+        }
     }
-        mPackageText.setText(sb.toString());
+
+    private void addText(String...string) {
+        for (String s : string) {
+            mPackageText.append(s);
+        }
+    }
+
+    private void addSectionTitle(String string) {
+        String s = mPackageText.getText();
+        int start = (s == null ? 0 : s.length());
+        mPackageText.append(string);
+
+        StyleRange sr = new StyleRange();
+        sr.start = start;
+        sr.length = string.length();
+        sr.fontStyle = SWT.BOLD;
+        sr.underline = true;
         mPackageText.setStyleRange(sr);
     }
 

tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdaterData.java

@@ -366,7 +366,7 @@ class UpdaterData {
 
     /**
      * Tries to update all the *existing* local packages.
-     * This first refreshes all sources, then compares the available remote packages when
+     * This first refreshes all sources, then compares the available remote packages with
      * the current local ones and suggest updates to be done to the user. Finally all
      * selected updates are installed.
      *
@@ -386,13 +386,13 @@ class UpdaterData {
             // selected archives. If they do not match an update, list them anyway
             // except they map themselves to null (no "old" archive)
             for (Archive a : selectedArchives) {
-                if (!updates.containsValue(a)) {
+                if (!updates.containsKey(a)) {
                     updates.put(a, null);
                 }
             }
         }
 
-        UpdateChooserDialog dialog = new UpdateChooserDialog(this, updates);
+        UpdateChooserDialog dialog = new UpdateChooserDialog(getWindowShell(), this, updates);
         dialog.open();
 
         Collection<Archive> result = dialog.getResult();
@@ -444,9 +444,9 @@ class UpdaterData {
         // Map [remote archive => local archive] of suitable update candidates
         Map<Archive, Archive> result = new HashMap<Archive, Archive>();
 
-        // First go thru all sources and make a local list of all available archives
+        // First go thru all sources and make a list of all available remote archives
         // sorted by package class.
-        HashMap<Class<? extends Package>, ArrayList<Archive>> availPkgs =
+        HashMap<Class<? extends Package>, ArrayList<Archive>> availablePkgs =
             new HashMap<Class<? extends Package>, ArrayList<Archive>>();
 
         if (selectedArchives != null) {
@@ -457,9 +457,9 @@ class UpdaterData {
                 if (a.isCompatible()) {
                     Class<? extends Package> clazz = a.getParentPackage().getClass();
 
-                    ArrayList<Archive> list = availPkgs.get(clazz);
+                    ArrayList<Archive> list = availablePkgs.get(clazz);
                     if (list == null) {
-                        availPkgs.put(clazz, list = new ArrayList<Archive>());
+                        availablePkgs.put(clazz, list = new ArrayList<Archive>());
                     }
 
                     list.add(a);
@@ -476,9 +476,9 @@ class UpdaterData {
                     for (Package remotePkg : remotePkgs) {
                         Class<? extends Package> clazz = remotePkg.getClass();
 
-                        ArrayList<Archive> list = availPkgs.get(clazz);
+                        ArrayList<Archive> list = availablePkgs.get(clazz);
                         if (list == null) {
-                            availPkgs.put(clazz, list = new ArrayList<Archive>());
+                            availablePkgs.put(clazz, list = new ArrayList<Archive>());
                         }
 
                         for (Archive a : remotePkg.getArchives()) {
@@ -500,7 +500,7 @@ class UpdaterData {
 
         for (Package localPkg : localPkgs) {
             // get the available archive list for this package type
-            ArrayList<Archive> list = availPkgs.get(localPkg.getClass());
+            ArrayList<Archive> list = availablePkgs.get(localPkg.getClass());
 
             // if this list is empty, we'll never find anything that matches
             if (list == null || list.size() == 0) {
@@ -511,7 +511,7 @@ class UpdaterData {
             Archive[] localArchives = localPkg.getArchives();
             if (localArchives != null && localArchives.length > 0) {
                 Archive localArchive = localArchives[0];
-                // only consider archive compatible with the current platform
+                // only consider archives compatible with the current platform
                 if (localArchive != null && localArchive.isCompatible()) {
 
                     // We checked all this archive stuff because that's what eventually gets