<AWT Dev> A patch so multiple selected files can have different directories.

Anthony Petrov anthony.petrov at oracle.com
Tue Jan 31 08:57:38 PST 2012


Hi Matthew,

The updated webrev is available at:

http://cr.openjdk.java.net/~anthony/8-10-GTKFileDialogMultiSelRecentFiles-7132194.1/

Please note that there's still problems with formatting at lines 210-211 
(TABs instead of spaces), 239 (missing spaces), and also an extra empty 
line at 248. No need to resend a new patch, I'll fix these myself before 
pushing the fix.

In the future, please pay attention to formatting your code. At first 
this issue may seem unimportant, however, it greatly simplifies 
maintaining the code, especially as large as OpenJDK. That's why we 
require all fixes to follow the common Java Code Conventions and be 
properly formatted. You could use the webrev utility [1] to generate 
webreves for your code changes. It is easier to spot mis-formatted code 
using a HTML view of your changes.

Having said that, I'm approving the fix. Thank you for the contribution! 
If anyone on the mailing list has additional comments, they are welcome.

[1] http://openjdk.java.net/guide/webrevHelp.html

--
best regards,
Anthony

On 1/30/2012 11:51 PM, Matthew Smith wrote:
> Anthony
> 
> I made the changes suggested and tried it out. Everything seems ok.
> 
> Cheers.
> mbs
> 
> On 01/30/2012 10:08 AM, Anthony Petrov wrote:
>> Hi Matthew,
>>
>> I've published a webrev at:
>>
>> http://cr.openjdk.java.net/~anthony/8-10-GTKFileDialogMultiSelRecentFiles-7132194.0/ 
>>
>>
>> I have a few stylistic comments:
>>
>> 1. Lines 187, 235: please avoid using reverse order for conditionals. 
>> I.e. (list == NULL) is preferred to (NULL == list).
>>
>> 2. Line 208: always put the if/else (and while()) statements into 
>> blocks {} (and add a new line after {.)
>>
>> 3. Lines 208, 235,236, 240, 246: please use proper spacing (e.g.. "if 
>> (cond) {", "a = b;", etc.)
>>
>> 4. (this one is less stylistic): at line 208 what prevents us from 
>> using "if (entry[0] == '/')"? This seems more simple and robust than 
>> using strchr() and pointer arithmetics.
>>
>> Other than that the fix looks fine to me, thank you! Once you fix the 
>> issues above and send us an updated patch, I can push the fix to the 
>> repository.
>>
>> Comments from other AWT team members (and anyone interested) are welcome!
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 01/24/12 00:50, Matthew Smith wrote:
>>> openjdk/jdk/src/solaris/native/sun/awt/sun_awt_X11_GtkFileDialogPeer.c
>>>
>>> This patch is intended to address the issue of newer versions of gtk
>>> where the file dialog lets you select files w/out selecting a directory
>>> via Search or Recently Used. When this happens the 'current directory'
>>> is returned as null, and the Files selected will be returned with the
>>> current working directory appended to the path.
>>>
>>> The .java file will show the incorrect behavior if you run it and use
>>> Recently Used to select a file (gnome 3).
>>>
>>> This patch does not change the behavior when a directory is selected, or
>>> when the dialog is canceled. If you select a file from the Recently Used
>>> option the directory will be set to the root directory, and the filename
>>> will be the complete path and filename. When multiple files are selected
>>> from a folder the original behavior occurs. If multiple files are
>>> selected from Recently Used then the directory is again set to root, and
>>> all of the fiIes have the complete name.
>>>
>>> This work around can be verified with the .java file provided.
>>>
>>> Finally this is an updated version of the patch, originally I used
>>> 'bool' instead of a 'gboolean' I did compile it and test that it works.
>>>
>>> Thank you
>>> mbs
>>
> 



More information about the awt-dev mailing list