Comment 65 for bug 586910

Revision history for this message
In , Lufimtse (lufimtse) wrote :

(In reply to Markus Keller from comment #58)
> (In reply to Eclipse Genie from comment #53)
> > Gerrit change https://git.eclipse.org/r/50125 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=3db87a8763e3fd3f931295da165ddee8363a7b16
> >
>
> The Widget#valueContainsFlag(..) doesn't meet SWT's coding style and quality
> standards:
>
> - The standard idiom in SWT is this:
>
> if ((keymask & (OS.GDK_MOD1_MASK | OS.GDK_SHIFT_MASK
> | OS.GDK_CONTROL_MASK | OS.GDK_SUPER_MASK
> | OS.GDK_META_MASK | OS.GDK_HYPER_MASK)) != 0) {
> sendTreeDefaultSelection();
> }
>
> Also note the position of the '{'.

Thank you for feedback. I've made the relevant refactoring and tested for functionality. (removed valueContainsFlag() method).
I added you as reviewer:
https://git.eclipse.org/r/#/c/51524/
Please let me know if there's anything else.

> The valueContainsFlag method should be removed again. If it would have been
> kept, the following would have had to be fixed:
>
> - Why is the type of 'value' long and not int?
> - Missing @return description.
> - Typo: Convienience (please check why you didn't see an error underline
> from the spell checker)
To this I shall link to this tweet:
https://twitter.com/iamdevloper/status/614509767747178499

(On the side: I did have spell check turned on, but it seems by default Eclipse only picks up the first 100 spell check errors. The function being at the bottom of a 2000+ widget, Eclipse didn't get to the typo. I set my preferences to 9999 instead now)