Skip to content

Commit

Permalink
SLE-921: Overhaul dialogs and handle cases
Browse files Browse the repository at this point in the history
Overhaul the dialogs to also show information when the suggestion change cannot be applied.

Handle the cases differently when the user cancels the action, when the suggestion cannot be applied.
  • Loading branch information
thahnen committed Aug 21, 2024
1 parent 6cc454f commit f44f713
Show file tree
Hide file tree
Showing 9 changed files with 209 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,13 @@ public static void addedAutomaticBindings() {
getTelemetryService().addedAutomaticBindings();
}

public static void handledFixSuggestion(String id, int changeIndex, boolean accepted) {
getTelemetryService().fixSuggestionResolved(new FixSuggestionResolvedParams(id,
accepted ? FixSuggestionStatus.ACCEPTED : FixSuggestionStatus.DECLINED,
changeIndex));
public static void acceptFixSuggestion(String id, int changeIndex) {
getTelemetryService().fixSuggestionResolved(
new FixSuggestionResolvedParams(id, FixSuggestionStatus.ACCEPTED, changeIndex));
}

public static void declineFixSuggestion(String id, int changeIndex) {
getTelemetryService().fixSuggestionResolved(
new FixSuggestionResolvedParams(id, FixSuggestionStatus.DECLINED, changeIndex));
}
}
Binary file added org.sonarlint.eclipse.ui/icons/warn.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public final class SonarLintImages {
public static final Image IMG_SONARCLOUD_LOGO = createImage("logo/sonarcloud-black-256px.png"); //$NON-NLS-1$

public static final Image IMG_OPEN_EXTERNAL = createImage("external-link-16.png"); //$NON-NLS-1$
public static final Image IMG_WARNING = createImage("warn.png"); //$NON-NLS-1$

public static final Image NOTIFICATION_CLOSE = createImage("notifications/eview16/notification-close.png"); //$NON-NLS-1$
public static final Image NOTIFICATION_CLOSE_HOVER = createImage("notifications/eview16/notification-close-active.png"); //$NON-NLS-1$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.util.Locale;
import org.eclipse.compare.CompareConfiguration;
import org.eclipse.compare.IEncodedStreamContentAccessor;
import org.eclipse.compare.ITypedElement;
Expand All @@ -42,34 +43,51 @@
import org.sonarlint.eclipse.core.analysis.SonarLintLanguage;
import org.sonarlint.eclipse.ui.internal.extension.SonarLintUiExtensionTracker;

public class FixSuggestionDialog extends Dialog {
public abstract class AbstractFixSuggestionDialog extends Dialog {
@Nullable
private final SonarLintLanguage language;
private final String explanation;
private final String textLeft;
private final String textRight;
private final String beforeText;
private final String afterText;
private final int changeIndex;
private final int absoluteNumberOfChanges;
private final CompareConfiguration mp;

public FixSuggestionDialog(Shell parentShell, @Nullable SonarLintLanguage language, String explanation,
String textLeft, String textRight) {
/**
* @param parentShell used for the dialog
* @param language used for determining the correct diff viewer for the language
* @param explanation shown about the suggestion to explain the diff
* @param beforeText what will be replaced
* @param afterText with what it will be replaced
* @param changeIndex the number of the index (-1)
* @param absoluteNumberOfChanges all number of changes
*/
protected AbstractFixSuggestionDialog(Shell parentShell, @Nullable SonarLintLanguage language, String explanation,
String beforeText, String afterText, int changeIndex, int absoluteNumberOfChanges) {
super(parentShell);

this.language = language;
this.explanation = explanation;
this.textLeft = textLeft;
this.textRight = textRight;
this.beforeText = beforeText;
this.afterText = afterText;
this.changeIndex = changeIndex;
this.absoluteNumberOfChanges = absoluteNumberOfChanges;
this.mp = new CompareConfiguration();
mp.setProperty(CompareConfiguration.MIRRORED, true);
mp.setLeftEditable(false);
mp.setLeftLabel("Current code");
mp.setLeftLabel("Suggested code");
mp.setRightEditable(false);
mp.setRightLabel("Suggested code");
mp.setRightLabel("Current code on the server");
}

@Override
protected Control createDialogArea(Composite parent) {
var container = (Composite) super.createDialogArea(parent);
container.setLayout(new GridLayout(1, true));

// So the sub-classes can add a specific label with situational information!
addLabel(container);

var label = new Label(container, SWT.WRAP);
label.setText(explanation);
label.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false));
Expand All @@ -80,22 +98,15 @@ protected Control createDialogArea(Composite parent) {
} else {
diffViewer = getDiffViewer(container);
}
diffViewer.setInput(new DiffNode(new CodeNode(textLeft), new CodeNode(textRight)));
var gridData = new GridData();
gridData.horizontalAlignment = SWT.FILL;
gridData.verticalAlignment = SWT.FILL;
gridData.grabExcessHorizontalSpace = true;
gridData.grabExcessVerticalSpace = true;
// INFO: Because we mirrored the diff viewer the right becomes left and the left becomes right
diffViewer.setInput(new DiffNode(new CodeNode(afterText), new CodeNode(beforeText)));
diffViewer.getControl().setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));

return container;
}

@Override
protected void createButtonsForButtonBar(Composite parent) {
createButton(parent, IDialogConstants.OK_ID, "Apply Changes", true);
createButton(parent, IDialogConstants.CANCEL_ID, "Cancel", false);
createButton(parent, IDialogConstants.SKIP_ID, "Decline Changes", false);
protected void addLabel(Composite container) {
// Should be overwritten by sub-classes that provide situational information!
}

/** Because the "skip" button is not implemented by default, we have to do it on our own! */
Expand All @@ -112,8 +123,8 @@ protected void buttonPressed(int buttonId) {
protected void configureShell(Shell newShell) {
super.configureShell(newShell);

newShell.setText("SonarLint Fix Suggestion");
newShell.setSize(800, 400);
newShell.setText(String.format("SonarLint Fix Suggestion (%d/%d)", changeIndex + 1, absoluteNumberOfChanges));
newShell.setSize(1000, 400);
}

@Override
Expand All @@ -122,7 +133,7 @@ protected boolean isResizable() {
}

private TextMergeViewer getDiffViewer(Composite parent) {
var fileLanguage = language.name().toLowerCase();
var fileLanguage = language.name().toLowerCase(Locale.getDefault());

var configurationProviders = SonarLintUiExtensionTracker.getInstance().getSyntaxHighlightingProvider();
for (var configurationProvider : configurationProviders) {
Expand All @@ -137,6 +148,10 @@ private TextMergeViewer getDiffViewer(Composite parent) {
return new TextMergeViewer(parent, mp);
}

/**
* The TextMergeViewer works with distinct nodes for each side, we have to implement it ourself as there is no
* common implementation that can be reused. It is basically fancy wrapper for a string -.-
*/
private static class CodeNode implements ITypedElement, IEncodedStreamContentAccessor {
private final String content;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* SonarLint for Eclipse
* Copyright (C) 2015-2024 SonarSource SA
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonarlint.eclipse.ui.internal.dialog;

import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jface.dialogs.IDialogConstants;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Shell;
import org.sonarlint.eclipse.core.analysis.SonarLintLanguage;

/** Version of the dialog where the fix suggestion is available! */
public class FixSuggestionAvailableDialog extends AbstractFixSuggestionDialog {
public FixSuggestionAvailableDialog(Shell parentShell, @Nullable SonarLintLanguage language, String explanation,
String textLeft, String textRight, int changeIndex, int absoluteNumberOfChanges) {
super(parentShell, language, explanation, textLeft, textRight, changeIndex, absoluteNumberOfChanges);
}

/**
* "Apply Changes" -> applies the change and moves on to the next one
* "Decline Changes" -> does not apply the change, but moves on to the next one
* "Cancel" -> cancels the whole process, nothing will be further applied
*
* The order is based on how it will be displayed in dialog. The first one is always the most right one (because of
* the "true") and the others are aligned to its left from left to right!
*/
@Override
protected void createButtonsForButtonBar(Composite parent) {
createButton(parent, IDialogConstants.OK_ID, "Apply Changes", true);
createButton(parent, IDialogConstants.CANCEL_ID, "Cancel", false);
createButton(parent, IDialogConstants.SKIP_ID, "Decline Changes", false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* SonarLint for Eclipse
* Copyright (C) 2015-2024 SonarSource SA
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonarlint.eclipse.ui.internal.dialog;

import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jface.dialogs.IDialogConstants;
import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.CLabel;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Shell;
import org.sonarlint.eclipse.core.analysis.SonarLintLanguage;
import org.sonarlint.eclipse.ui.internal.SonarLintImages;

/** Version of the dialog where the fix suggestion is not available due to it not being found! */
public class FixSuggestionUnavailableDialog extends AbstractFixSuggestionDialog {
public FixSuggestionUnavailableDialog(Shell parentShell, @Nullable SonarLintLanguage language, String explanation,
String textLeft, String textRight, int changeIndex, int absoluteNumberOfChanges) {
super(parentShell, language, explanation, textLeft, textRight, changeIndex, absoluteNumberOfChanges);
}

/**
* "Proceed" -> does not apply the change and moves on to the next one
* "Cancel" -> cancels the whole process, nothing will be further applied
*
* The order is based on how it will be displayed in dialog. The first one is always the most right one (because of
* the "true") and the others are aligned to its left from left to right!
*/
@Override
protected void createButtonsForButtonBar(Composite parent) {
createButton(parent, IDialogConstants.OK_ID, "Proceed", true);
createButton(parent, IDialogConstants.CANCEL_ID, "Cancel", false);
}

@Override
protected void addLabel(Composite container) {
var label = new CLabel(container, SWT.WRAP);
label.setText("This change suggestion is not applicable as the current code cannot be found in the file.");
// INFO: When resized to be too small, this icon won't be shown anymore!
label.setImage(SonarLintImages.IMG_WARNING);
label.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,15 @@ public abstract class AbstractOpenInEclipseJob extends Job {
protected ISonarLintFile file;

protected final ISonarLintProject project;
private final boolean skipAnalysisReadyCheck;
private final boolean skipBranchCheck;

protected AbstractOpenInEclipseJob(String name, ISonarLintProject project, boolean skipBranchCheck) {
protected AbstractOpenInEclipseJob(String name, ISonarLintProject project, boolean skipAnalysisReadyCheck,
boolean skipBranchCheck) {
super(name);

this.project = project;
this.skipAnalysisReadyCheck = skipAnalysisReadyCheck;
this.skipBranchCheck = skipBranchCheck;
}

Expand All @@ -68,7 +71,9 @@ protected IStatus run(IProgressMonitor monitor) {
// 1) We have to await the analysis getting ready for this project. When also setting up the connection / binding
// with this Open in IDE request it isn't the case. We want to display a nice dialog informing the user that they
// have to wait for a few more moments.
if (!AnalysisReadyStatusCache.getAnalysisReadiness(ConfigScopeSynchronizer.getConfigScopeId(project))) {
// -> When the check is not needed, the whole stuff gets skipped!
if (!skipAnalysisReadyCheck
&& !AnalysisReadyStatusCache.getAnalysisReadiness(ConfigScopeSynchronizer.getConfigScopeId(project))) {
// Show the console so the user will see the progress!
SonarLintUiPlugin.getDefault().getSonarConsole().bringConsoleToFront();

Expand Down
Loading

0 comments on commit f44f713

Please sign in to comment.