From 6c9309466b6f220fd777c0605f2a49caabf66310 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 7 Feb 2021 16:30:02 +0100 Subject: [PATCH] EGit UI DebugOptionsListener as a OSGi declarative service Remove the DebugOptionsListener from the Activator and make it a separate OSGi declarative service. Let the framework handle subscribing and unsubscribing this service. Adapt the GitTraceConfigurationDialog, and fix two bugs: * If the trace location is set to stdout in the PDE trace preferences, the trace file may be null. Disable the field and the "Open in Editor" button in that case. * Make the dialog contents always fill the whole available space. Three notes: 1. The EGit GitTraceConfigurationDialog does not interact well with the PDE trace preference page. While EGit picks up changes done via the PDE preference page, that preference page does _not_ pick up changes done via the EGit dialog. 2. Using a static inner class GitTraceLocation$DebugOptionsHandler did not work in Eclipse Neon.3. With the DebugOptionsHandler as a separate top-level class it works. 3. The command to open the EGit trace dialog is a bit hidden; it does not appear in any menu, and has no key binding. A user can define a key binding for it in the Eclipse preferences, though. The command is named "Configure Git Debug Trace". Change-Id: Icfcee3eeb27fe98a92566656c4b7b0c19cf088af Signed-off-by: Thomas Wolf --- .../test/trace/TraceConfigurationDialogTest.java | 6 ++-- org.eclipse.egit.ui/META-INF/MANIFEST.MF | 3 +- ....egit.ui.internal.trace.DebugOptionsHandler.xml | 8 +++++ .../src/org/eclipse/egit/ui/Activator.java | 42 +++------------------- .../dialogs/GitTraceConfigurationDialog.java | 17 +++++++-- .../ui/internal/trace/DebugOptionsHandler.java | 29 +++++++++++++++ .../egit/ui/internal/trace/GitTraceLocation.java | 29 ++++++++++----- 7 files changed, 82 insertions(+), 52 deletions(-) create mode 100644 org.eclipse.egit.ui/OSGI-INF/org.eclipse.egit.ui.internal.trace.DebugOptionsHandler.xml create mode 100644 org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/DebugOptionsHandler.java diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/trace/TraceConfigurationDialogTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/trace/TraceConfigurationDialogTest.java index 5f7da3baa..a4d806830 100644 --- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/trace/TraceConfigurationDialogTest.java +++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/trace/TraceConfigurationDialogTest.java @@ -15,10 +15,10 @@ package org.eclipse.egit.ui.test.trace; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import org.eclipse.egit.ui.Activator; import org.eclipse.egit.ui.common.EGitTestCase; import org.eclipse.egit.ui.internal.UIText; import org.eclipse.egit.ui.internal.dialogs.GitTraceConfigurationDialog; +import org.eclipse.egit.ui.internal.trace.GitTraceLocation; import org.eclipse.jface.dialogs.IDialogConstants; import org.eclipse.swt.widgets.Shell; import org.eclipse.swtbot.eclipse.finder.SWTWorkbenchBot; @@ -45,13 +45,13 @@ public class TraceConfigurationDialogTest { public static void beforeClass() throws Exception { EGitTestCase.closeWelcomePage(); // make sure tracing is off globally - Activator.getDefault().getDebugOptions().setDebugEnabled(false); + GitTraceLocation.getOptions().setDebugEnabled(false); } @AfterClass public static void afterClass() throws Exception { // make sure tracing is off globally - Activator.getDefault().getDebugOptions().setDebugEnabled(false); + GitTraceLocation.getOptions().setDebugEnabled(false); } @Before diff --git a/org.eclipse.egit.ui/META-INF/MANIFEST.MF b/org.eclipse.egit.ui/META-INF/MANIFEST.MF index 4c5f91fac..a1814406f 100644 --- a/org.eclipse.egit.ui/META-INF/MANIFEST.MF +++ b/org.eclipse.egit.ui/META-INF/MANIFEST.MF @@ -133,4 +133,5 @@ Export-Package: org.eclipse.egit.ui;version="5.11.0";x-friends:="org.eclipse.egi Service-Component: OSGI-INF/org.eclipse.egit.ui.internal.clone.GitCloneDropAdapter.xml, OSGI-INF/org.eclipse.egit.ui.internal.StartEventListener.xml, OSGI-INF/org.eclipse.egit.ui.internal.ApplicationActiveListener.xml, - OSGI-INF/org.eclipse.egit.ui.internal.ExternalRepositoryScanner.xml + OSGI-INF/org.eclipse.egit.ui.internal.ExternalRepositoryScanner.xml, + OSGI-INF/org.eclipse.egit.ui.internal.trace.DebugOptionsHandler.xml diff --git a/org.eclipse.egit.ui/OSGI-INF/org.eclipse.egit.ui.internal.trace.DebugOptionsHandler.xml b/org.eclipse.egit.ui/OSGI-INF/org.eclipse.egit.ui.internal.trace.DebugOptionsHandler.xml new file mode 100644 index 000000000..07d9e00b1 --- /dev/null +++ b/org.eclipse.egit.ui/OSGI-INF/org.eclipse.egit.ui.internal.trace.DebugOptionsHandler.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java index e0cd8fe95..6d27527ec 100755 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java @@ -5,7 +5,7 @@ * Copyright (C) 2012, Matthias Sohn * Copyright (C) 2015, Philipp Bumann * Copyright (C) 2016, Dani Megert - * Copyright (C) 2018, Thomas Wolf + * Copyright (C) 2018, 2021 Thomas Wolf * * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -18,8 +18,6 @@ package org.eclipse.egit.ui; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; -import java.util.Dictionary; -import java.util.Hashtable; import java.util.Iterator; import java.util.List; @@ -32,7 +30,6 @@ import org.eclipse.egit.ui.internal.ConfigurationChecker; import org.eclipse.egit.ui.internal.KnownHosts; import org.eclipse.egit.ui.internal.UIText; import org.eclipse.egit.ui.internal.credentials.EGitCredentialsProvider; -import org.eclipse.egit.ui.internal.trace.GitTraceLocation; import org.eclipse.egit.ui.internal.variables.GitTemplateVariableResolver; import org.eclipse.jdt.internal.ui.JavaPlugin; import org.eclipse.jface.resource.JFaceResources; @@ -43,8 +40,6 @@ import org.eclipse.jface.text.templates.TemplateContextType; import org.eclipse.jface.util.IPropertyChangeListener; import org.eclipse.jface.util.PropertyChangeEvent; import org.eclipse.jgit.transport.CredentialsProvider; -import org.eclipse.osgi.service.debug.DebugOptions; -import org.eclipse.osgi.service.debug.DebugOptionsListener; import org.eclipse.swt.graphics.Font; import org.eclipse.swt.widgets.Display; import org.eclipse.ui.PlatformUI; @@ -53,12 +48,14 @@ import org.eclipse.ui.progress.WorkbenchJob; import org.eclipse.ui.statushandlers.StatusManager; import org.eclipse.ui.themes.ITheme; import org.osgi.framework.BundleContext; -import org.osgi.framework.ServiceRegistration; /** * This is a plugin singleton mostly controlling logging. */ -public class Activator extends AbstractUIPlugin implements DebugOptionsListener { +public class Activator extends AbstractUIPlugin { + + /** The plug-in ID for the EGit UI. */ + public static final String PLUGIN_ID = "org.eclipse.egit.ui"; //$NON-NLS-1$ /** * The one and only instance @@ -286,10 +283,6 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener private ResourceManager resourceManager; - private DebugOptions debugOptions; - - private ServiceRegistration serviceRegistration; - /** * Construct the {@link Activator} egit ui plugin singleton instance */ @@ -305,13 +298,6 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener @Override public void start(final BundleContext context) throws Exception { super.start(context); - // we want to be notified about debug options changes - Dictionary props = new Hashtable<>(4); - props.put(DebugOptions.LISTENER_SYMBOLICNAME, context.getBundle() - .getSymbolicName()); - serviceRegistration = context.registerService( - DebugOptionsListener.class.getName(), this, - props); setupCredentialsProvider(); ConfigurationChecker.checkConfiguration(); @@ -359,20 +345,6 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener job.schedule(); } - @Override - public void optionsChanged(DebugOptions options) { - // initialize the trace stuff - debugOptions = options; - GitTraceLocation.initializeFromOptions(options, isDebugging()); - } - - /** - * @return the {@link DebugOptions} - */ - public DebugOptions getDebugOptions() { - return debugOptions; - } - /** * Register for changes made to Team properties. * @@ -408,10 +380,6 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener @Override public void stop(final BundleContext context) throws Exception { - if (serviceRegistration != null) { - serviceRegistration.unregister(); - serviceRegistration = null; - } if (resourceManager != null) { resourceManager.dispose(); resourceManager = null; diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/GitTraceConfigurationDialog.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/GitTraceConfigurationDialog.java index 923508adc..dc70d1537 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/GitTraceConfigurationDialog.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/GitTraceConfigurationDialog.java @@ -10,6 +10,7 @@ *******************************************************************************/ package org.eclipse.egit.ui.internal.dialogs; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.net.URL; @@ -28,6 +29,7 @@ import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Platform; import org.eclipse.egit.ui.Activator; import org.eclipse.egit.ui.internal.UIText; +import org.eclipse.egit.ui.internal.trace.GitTraceLocation; import org.eclipse.jface.dialogs.Dialog; import org.eclipse.jface.dialogs.IDialogConstants; import org.eclipse.jface.dialogs.IMessageProvider; @@ -240,6 +242,8 @@ public class GitTraceConfigurationDialog extends TitleAreaDialog { private Text traceFileLocation; + private Button openButton; + private CheckboxTreeViewer tv; /** @@ -253,6 +257,7 @@ public class GitTraceConfigurationDialog extends TitleAreaDialog { @Override protected Control createDialogArea(Composite parent) { Composite main = new Composite(parent, SWT.NONE); + GridDataFactory.fillDefaults().grab(true, false).applyTo(main); main.setLayout(new GridLayout(3, false)); platformSwitch = new Button(main, SWT.CHECK); @@ -289,7 +294,7 @@ public class GitTraceConfigurationDialog extends TitleAreaDialog { GridDataFactory.defaultsFor(traceFileLocation).grab(true, false) .applyTo(traceFileLocation); - Button openButton = new Button(main, SWT.PUSH); + openButton = new Button(main, SWT.PUSH); openButton .setText(UIText.GitTraceConfigurationDialog_OpenInEditorButton); openButton.addSelectionListener(new SelectionAdapter() { @@ -389,7 +394,13 @@ public class GitTraceConfigurationDialog extends TitleAreaDialog { platformSwitch.setSelection(options.isDebugEnabled()); } - traceFileLocation.setText(getOptions().getFile().getPath()); + File traceFile = getOptions().getFile(); + if (traceFile == null) { + traceFileLocation.setEnabled(false); + openButton.setEnabled(false); + } else { + traceFileLocation.setText(traceFile.getPath()); + } updateEnablement(); } @@ -552,6 +563,6 @@ public class GitTraceConfigurationDialog extends TitleAreaDialog { } private DebugOptions getOptions() { - return Activator.getDefault().getDebugOptions(); + return GitTraceLocation.getOptions(); } } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/DebugOptionsHandler.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/DebugOptionsHandler.java new file mode 100644 index 000000000..d6d4dce73 --- /dev/null +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/DebugOptionsHandler.java @@ -0,0 +1,29 @@ +/******************************************************************************* + * Copyright (c) 2021 Thomas Wolf and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.egit.ui.internal.trace; + +import org.eclipse.egit.ui.Activator; +import org.eclipse.osgi.service.debug.DebugOptions; +import org.eclipse.osgi.service.debug.DebugOptionsListener; +import org.osgi.service.component.annotations.Component; + +/** + * OSGi component to get notified about debug option changes. + */ +@Component(property = DebugOptions.LISTENER_SYMBOLICNAME + '=' + + Activator.PLUGIN_ID) +public class DebugOptionsHandler implements DebugOptionsListener { + + @Override + public void optionsChanged(DebugOptions options) { + GitTraceLocation.initializeFromOptions(options); + } +} \ No newline at end of file diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/GitTraceLocation.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/GitTraceLocation.java index 1a2a3bc56..9c961370e 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/GitTraceLocation.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/trace/GitTraceLocation.java @@ -1,5 +1,6 @@ /******************************************************************************* - * Copyright (c) 2010 SAP AG. + * Copyright (c) 2010, 2021 SAP AG and others. + * * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -42,14 +43,15 @@ public enum GitTraceLocation implements ITraceLocation { * Initialize the locations * * @param options - * @param pluginIsDebugging + * to initialize from */ - public static void initializeFromOptions(DebugOptions options, - boolean pluginIsDebugging) { - + public static void initializeFromOptions(DebugOptions options) { + currentOptions = options; // we evaluate the plug-in switch + boolean pluginIsDebugging = options + .getBooleanOption(Activator.PLUGIN_ID + "/debug", false); //$NON-NLS-1$ if (pluginIsDebugging) { - myTrace = options.newDebugTrace(Activator.getPluginId()); + myTrace = options.newDebugTrace(Activator.PLUGIN_ID); for (GitTraceLocation loc : values()) { boolean active = options.getBooleanOption(loc.getFullPath(), @@ -65,16 +67,27 @@ public enum GitTraceLocation implements ITraceLocation { } } + /** + * Retrieves the current global {@link DebugOptions}. + * + * @return the {@link DebugOptions} + */ + public static DebugOptions getOptions() { + return currentOptions; + } + private final String location; private final String fullPath; private boolean active = false; - private static DebugTrace myTrace; + private static volatile DebugTrace myTrace; + + private static volatile DebugOptions currentOptions; private GitTraceLocation(String path) { - this.fullPath = Activator.getPluginId() + path; + this.fullPath = Activator.PLUGIN_ID + path; this.location = path; } -- 2.11.4.GIT