Skip to content

Conversation

@rishwanth1995
Copy link
Contributor

lookandfeel is set to crossplatform instead of native look and feel.


private void setUnixLookAndFeel(){
try {
//UIManager.setLookAndFeel(UIManager.getCrossPlatformLookAndFeelClassName());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a duplicate line, commented out?

//UIManager.setLookAndFeel(UIManager.getCrossPlatformLookAndFeelClassName());
UIManager.setLookAndFeel(UIManager.getCrossPlatformLookAndFeelClassName());
} catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
Exceptions.printStackTrace(ex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please log the error, don't print it to stdout.

What happens to the LAF if an exception is thrown here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rishwanth1995 says that if there is an exception, the native look and feel will be used, which will cause the problem we are trying to solve to manifest anyway. Perhaps we want to have a new setting to indicate whether or not it is "safe" to run the Timelline tool. Perhaps it is not worth it? @bcarrier, what are your thoughts on this?

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 we can solve that in the next release. This is good enough for now.

import javax.swing.UnsupportedLookAndFeelException;
import org.netbeans.swing.tabcontrol.plaf.DefaultTabbedContainerUI;
import org.openide.modules.ModuleInstall;
import org.openide.util.Exceptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

This import is not needed, see below.

//UIManager.setLookAndFeel(UIManager.getCrossPlatformLookAndFeelClassName());
UIManager.setLookAndFeel(UIManager.getCrossPlatformLookAndFeelClassName());
} catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
Exceptions.printStackTrace(ex);
Copy link
Contributor

Choose a reason for hiding this comment

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

@rishwanth1995 says that if there is an exception, the native look and feel will be used, which will cause the problem we are trying to solve to manifest anyway. Perhaps we want to have a new setting to indicate whether or not it is "safe" to run the Timelline tool. Perhaps it is not worth it? @bcarrier, what are your thoughts on this?

private static final long serialVersionUID = 1L;
private static final Logger logger = Logger.getLogger(Installer.class.getName());
private static Installer instance;
public static boolean enableTimeline = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Our coding guidelines prohibit global variables. Please put this value in a properties file for Timeline to read. There is a ModuleSettings utility class that could be used for managing the properties file.

Note that you will need to reset the property to enabled at the beginning of this set LAF operation.

enableTimeline=false;
} catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex1) {
logger.log(Level.WARNING, "Error setting native look-and-feel",ex1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that you say that the default system LAF will be used anyway, I think we should reduce code complexity here (i.e., eliminate the nested try-catch) and simply set the property to false after logging the error.

@rcordovano rcordovano merged commit ad3786f into sleuthkit:release-4.6.0 Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants