apache / jmeter

Apache JMeter open-source load testing tool for analyzing and measuring the performance of a variety of services
https://jmeter.apache.org/
Apache License 2.0
8.14k stars 2.06k forks source link

Use System default Look And Feel (especially Feel) #2227

Closed asfimport closed 15 years ago

asfimport commented 15 years ago

Frank Caputo (Bug 47064): I patched JMeter to use the system look and feel by default and allow ctrl-click on the mac and use the apple-key for keyboard shortcuts of menu items.

All changes are platform independent. Instead of KeyEvent.CTRL_DOWN_MASK use Toolkit.getDefaultToolkit().getMenuShortcutKeyMask().

isRightClick also checks e.isPopupTrigger().

attached is the patchfile for 2.3.2.

Created attachment jmeter.diff: svn patch file

OS: All

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): Thanks for the patch.

Unfortunately, the patch does not seem to work for me - most of the changes are rejected. I don't know why that is.

Also, there is a Mac-specific change to jmeter.sh - are you sure that won't affect non-Mac Unixes? Can you generate a patch that detects the Mac?

asfimport commented 15 years ago

Frank Caputo (migrated from Bugzilla): (In reply to comment 1)

Thanks for the patch.

no problem.

Unfortunately, the patch does not seem to work for me - most of the changes are rejected. I don't know why that is.

for me the patch works:

svn co http://svn.apache.org/repos/asf/jakarta/jmeter/tags/v2_3_2 cd v2_3_2 patch -p0 -i ~/Desktop/jmeter.diff ant

ant compiles fine without errors.

Also, there is a Mac-specific change to jmeter.sh - are you sure that won't affect non-Mac Unixes? Can you generate a patch that detects the Mac?

i simply set a system property which is simply ignored on other systems than the mac.

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): It works for me against the tag; however the current code in trunk has moved on a lot since then, which is presumably why the patch does not work against it.

I suppose I can just do the replacements manually.

If I create a new nightly build with the changes, would you be able to test it out? I don't have a Mac to test on.

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): The System LAF on Windows is not as nice as the Metal LAF, and I'm not prepared to change all the LAFs just to support Macs.

I think one solution would be to allow the property "jmeter.laf" to have a local system override, e.g. "jmeter.laf.<os.name>" where <os.name> is obtained from the system at run-time.

JMeter would look for "jmeter.laf.<os.name>", then "jmeter.laf".

This would allow one to change the default just for Macs.

What does Java return for os.name on Macs? This is shown in the jmeter.log file.

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): Applied most of the changes:

URL: http://svn.apache.org/viewvc?rev=768370&view=rev Log: https://github.com/apache/jmeter/issues/2227 - fixes for Mac LAF

These are in the nightly builds starting with r768370.

Please can you try it out, and post details of the os.name.

asfimport commented 15 years ago

@milamberspace (migrated from Bugzilla): With New Revision 768370, JMeter doesn't start on Linux (ubuntu 9.04), because os.name is "linux" without space like "windows xp"

jmeter.log : 2009/04/24 20:23:38 FATAL - jmeter.JMeter: An error occurred: java.lang.ExceptionInInitializerError at java.lang.Class.forName0(Native Method) at java.lang.Class.forName(Class.java:169) at org.apache.jmeter.gui.action.ActionRouter.populateCommandMap(ActionRouter.java:275) at org.apache.jmeter.gui.action.ActionRouter.getInstance(ActionRouter.java:308) at org.apache.jmeter.JMeter.startGui(JMeter.java:220) at org.apache.jmeter.JMeter.start(JMeter.java:343) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.apache.jmeter.NewDriver.main(NewDriver.java:207) Caused by: java.lang.NullPointerException at org.apache.jmeter.gui.action.LookAndFeelCommand.getJMeterLaf(LookAndFeelCommand.java:77) at org.apache.jmeter.gui.action.LookAndFeelCommand.<clinit>(LookAndFeelCommand.java:49) ... 11 more

Created attachment proposal_patch_update_47064.txt: Patch to work with os.name = "linux"

proposal_patch_update_47064.txt ````diff Index: /Workspaces-JMeter/Jmeter/src/core/org/apache/jmeter/gui/action/LookAndFeelCommand.java =================================================================== --- /Workspaces-JMeter/Jmeter/src/core/org/apache/jmeter/gui/action/LookAndFeelCommand.java (revision 768370) +++ /Workspaces-JMeter/Jmeter/src/core/org/apache/jmeter/gui/action/LookAndFeelCommand.java (working copy) @@ -73,8 +73,8 @@ if (laf != null) { return laf; } - String osFamily = osName.substring(0, osName.indexOf(' '));// e.g. windows xp => windows - laf = JMeterUtils.getProperty(JMETER_LAF+"."+osFamily); + String[] osFamily = osName.split("\\s"); // e.g. windows xp => windows + laf = JMeterUtils.getProperty(JMETER_LAF+"."+osFamily[0]); if (laf != null) { return laf; } ````
asfimport commented 15 years ago

@milamberspace (migrated from Bugzilla): Note: I don't know if "System.getProperty("os.name")" always return a correct string, but if it's return null, there are a possible NPE.

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): Thanks for report and patch, applied in:

URL: http://svn.apache.org/viewvc?rev=768414&view=rev Log: Oops - avoid NPE if os.name has no space in it. Props to Milamber, see https://github.com/apache/jmeter/issues/2227

I'll update the nightly build shortly.

I don't think os.name should ever return null ...

asfimport commented 15 years ago

Frank Caputo (migrated from Bugzilla): (In reply to comment 4)

The System LAF on Windows is not as nice as the Metal LAF, and I'm not prepared to change all the LAFs just to support Macs.

I think one solution would be to allow the property "jmeter.laf" to have a local system override, e.g. "jmeter.laf.<os.name>" where <os.name> is obtained from the system at run-time.

JMeter would look for "jmeter.laf.<os.name>", then "jmeter.laf".

This would allow one to change the default just for Macs.

What does Java return for os.name on Macs? This is shown in the jmeter.log file.

Mac OS X returns "Mac OS X" for "os.name".

I think you should do something like:

-------- code ------------------------------------

// make os.name lower case and replace alt whitespaces with underscores String osName = System.getProperty("os.name").toLowerCase().replaceAll("\s", "_"); String laf = System.getProperty("jmeter.laf");

String osLaf = System.getProperty("jmeter.laf." + osName); if(osLaf != null) { laf = osLaf; }

-------- code ------------------------------------

Here are some interesting links for os.name: http://developer.apple.com/technotes/tn2002/tn2110.html http://mindprod.com/jgloss/properties.html#OSNAME

I will test the next nightly.

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): I think this is now finally fixed.

I've added the ability to set the LAF to System or CrossPlatform, as well as to individual LAF implementations:

URL: http://svn.apache.org/viewvc?rev=770777&view=rev Log: https://github.com/apache/jmeter/issues/2227 - Use System default Look And Feel (Mac)

Please re-open if the nightlies after r770777 don't work as expected on a Mac.

asfimport commented 15 years ago

Frank Caputo (migrated from Bugzilla): I tried r772972 and it works fine out of the box on my mac.

asfimport commented 15 years ago

Sebb (migrated from Bugzilla): Great - thanks for confirming this.