Comment 1 Deepak Azad 2009-10-21 08:48:17 EDT

Created attachment 150103 [details]
Patch

Added the comment using createNoteComposite(Font, Composite, String, String). This method has been copied over from PreferencePage class.

Comment 2 Dani Megert 2009-10-22 04:18:39 EDT

Thanks for the patch Deepak. There are some UI and some coding style issues:

- the note needs to be closer to the preference it's meant for
- the note needs to have same vertical alignment as the preference it's meant for
- the note needs to be disabled when the preference is disabled (see e.g. Compiler
  > Javadoc pref page
- the message should get a 'the'
  ==> Changing the reference source does not update open editors.

- even if we copy a method we make sure that our formatting and style is applied.
  The copied method e.g. uses spaces as indent which should be replaced by tab.
  HINT: You can avoid that by enabling Java > Editor > Save Actions (see attached
        picture.

- we generally avoid shared strings which the name of 
   TextEditorMessages.EditorsPlugin_note could indicate. Please scope it to the
   dialog where it's used
   ==> TextEditorMessages.QuickDiffConfigurationBlock_referenceProviderNoteTitle
   (and I would change referenceProviderNote to referenceProviderNoteMessage)

Comment 3 Dani Megert 2009-10-22 04:20:47 EDT

Created attachment 150207 [details]
My Save Actions

Comment 4 Deepak Azad 2009-10-22 09:02:41 EDT

Created attachment 150237 [details]
reworked patch with the suggested changes

Comment 5 Dani Megert 2009-10-23 09:56:03 EDT

Looks good. Some minor issues in the code:

1) it is not recommended to use new GridData(int)
2) first Javadoc sentence should end with a '.' (fQuickDiffProviderNote)
3) when we add a comment to a new method we also add "@since 3.6", even if it's
   not Javadoc

I've fixed 2 and 3 and committed the code to HEAD.
Available in builds > N20091022-2000.

There are several (also old) usages of new GridData(int) in this class. Can you please file a bug report for this and attach a patch that cleans this up (just in this class, there are tons of such usages all over the place but we clean them up whenever we see it).

Thanks!

Comment 6 Raksha Vasisht 2009-10-27 04:19:35 EDT

Verified for 3.6 M3 with I20091026-1800.