Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide static acessors in the ILog interface as an alternative to Platform #498

Merged
merged 1 commit into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-Version: 3.27.100.qualifier
Bundle-Version: 3.28.0.qualifier
Bundle-SymbolicName: org.eclipse.core.runtime; singleton:=true
Bundle-Vendor: %providerName
Bundle-Activator: org.eclipse.core.internal.runtime.PlatformActivator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public final class InternalPlatform {

private static final String[] ARCH_LIST = { Platform.ARCH_AARCH64, Platform.ARCH_X86, Platform.ARCH_X86_64 };

public static final StackWalker STACK_WALKER = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);

// debug support: set in loadOptions()
public static boolean DEBUG = false;
public static boolean DEBUG_PLUGIN_PREFERENCES = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
*******************************************************************************/
package org.eclipse.core.runtime;

import org.eclipse.core.internal.runtime.InternalPlatform;
import org.osgi.framework.Bundle;
import org.osgi.framework.FrameworkUtil;

/**
* A log to which status events can be written. Logs appear on individual
Expand Down Expand Up @@ -132,4 +134,45 @@ default void error(String message) {
default void error(String message, Throwable throwable) {
log(new Status(IStatus.ERROR, getBundle().getSymbolicName(), message, throwable));
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not it that it really matters, but I have the impression that usually such static factories methods are added at the top of the file instead of the bottom.

/**
* Returns the log for the given bundle. If no such log exists, one is created.
*
* @param bundle the bundle whose log is returned
* @return the log for the given bundle
* @since 3.28
*/
public static ILog of(Bundle bundle) {
return InternalPlatform.getDefault().getLog(bundle);
}

/**
* Returns the log for the bundle of the given class. If no such log exists, one
* is created.
*
* @param clazz the class in a bundle whose log is returned
* @return the log for the bundle to which the clazz belongs
*
* @since 3.28
*/
public static ILog of(Class<?> clazz) {
Bundle bundle = FrameworkUtil.getBundle(clazz);
return InternalPlatform.getDefault().getLog(bundle);
}

/**
* Returns the log for the bundle of the calling class. If no such log exists,
* one is created.
*
* @return the log for the bundle to which the caller belongs
*
* @since 3.28
*/
public static ILog get() {
try {
return of(InternalPlatform.STACK_WALKER.getCallerClass());
} catch (IllegalCallerException e) {
return of(ILog.class);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ public static long getStateStamp() {
* @since 3.0
*/
public static ILog getLog(Bundle bundle) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a comment mentioning the new method would be good here and below.
Eventually this method should at least be deprecated.

return InternalPlatform.getDefault().getLog(bundle);
return ILog.of(bundle);
}

/**
Expand All @@ -935,8 +935,7 @@ public static ILog getLog(Bundle bundle) {
* @since 3.16
*/
public static ILog getLog(Class<?> clazz) {
Bundle bundle = FrameworkUtil.getBundle(clazz);
return InternalPlatform.getDefault().getLog(bundle);
return ILog.of(clazz);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,13 +230,15 @@ public final URL find(IPath path, Map<String,String> override) {


/**
* Returns the log for this plug-in. If no such log exists, one is created.
* Returns the log for this plug-in. If no such log exists, one is created.
* <b>Hint: </b> instead of caling this method, consider using
* {@link ILog#of(Class)} instead that is independent from implementing a
* {@link Plugin}
*
* @return the log for this plug-in
* XXX change this into a LogMgr service that would keep track of the map. See if it can be a service factory.
*/
public final ILog getLog() {
return InternalPlatform.getDefault().getLog(getBundle());
return ILog.of(getBundle());
}

/**
Expand Down Expand Up @@ -354,7 +356,7 @@ public final void savePluginPreferences() {
// need to save them because someone else might have
// made changes via the OSGi APIs.
getPluginPreferences();

// Performance: isolate PreferenceForwarder and BackingStoreException into
// an inner class to avoid class loading (and then activation of the Preferences plugin)
// as the Plugin class is loaded.
Expand Down