Community
Participate
Working Groups
The SWT Button template does not inluded a SelectionListener. I never use a Button without a SelectionListener. If you want, I can add this to the template and provide a patch. If changing the template is not desired, please close this bug.
(In reply to comment #0) > The SWT Button template does not inluded a SelectionListener. I never use a > Button without a SelectionListener. Makes sense. > If you want, I can add this to the template and provide a patch. That would be nice. Please go over the SWT templates and also add other things you think make sense, e.g. selection listener for 'Link' template.
I also see that the Text template uses SWT.LEAD. AFAIK SWT.LEAD is not used in Text. Ok to remove this?
(In reply to comment #2) > I also see that the Text template uses SWT.LEAD. AFAIK SWT.LEAD is not used > in Text. Ok to remove this? yes.
Created attachment 227964 [details] Template change for button Attached the proposed template change for the button. It adds the selectionListener. It also puts quotes around the wordselection, as I think a word selection is rarely used and this avoids a syntax error after using the template. If you ok with this change, I change Link and Text.
Created attachment 227965 [details] Text template change Removes SWT.LEAD from Text and wraps word selection into "" to avoid syntax error if no word selection is done (standard case IMHO)
Created attachment 227966 [details] Link template patch Link template patch
If the change with the quotes around the wordselection is ok for you I adjust all other templates. If not let me know, I can change the patches. These patches try to be in the spirit of Git really small and isolated, if you prefer one patch for everything also let me know. I must say that the wordselection patch would make my life easier, as the templates would not create syntax errors anymore.
Any feedback on the patches? I'm particular found of the change that avoids the syntax error after using the template. If you agree with this change, I adjust the remaining templates.
(In reply to comment #8) > Any feedback on the patches? I'm particular found of the change that avoids > the syntax error after using the template. If you agree with this change, I > adjust the remaining templates. Sorry, but we are currently very busy with other work. I'll try to look at it before M7, but no promise.
Thanks Dani, if it helps the patch is trivial and I would like to know if you agree with the change that avoids the syntax error because if you do, I would need to adjust the other templates.
I like the change regarding the quotes. The link template change seems incomplete as it missed some of the required imports (SelectionEvent, SelectionAdapter). NOTE: If I get new patches before Monday 18:00 CEST I will try to put it into 4.3, otherwise this will have to be deferred to 4.4.
Thanks for the feedback. I try to update all templates accordingly before this tied deadline. :-)
Internal note: all tests pass with the 3 proposed patches.
TableColumn also only allows to use LEFT, RIGHT, CENTER according to its Javadoc. I included that also fix also in the patch.
Same for TreeColumn
Created attachment 230235 [details] Patch for the SWT templates Updated patch with avoids getting a syntax errors for all templates (usage of "") and adds selection listener to Link and Button. It also removes the usage of SWT.LEAD and SWT.TRAIL for Text, TableColumn and TreeColumn I tested if this patch applies with EGit and it did for me based on commit 29b40ac, please let me know if you encounter any issues.
Your deadline is approaching. Are you ok with the suggested patch or do you require changes?
After a second though I decided to only take the selection listener addition: - LEAD is not used often in our code, but it is the correct/preferred thing to use in newer applications, see SWT.LEAD and SWT.LEFT for details. And also bug 401291. - Using quotes looks helpful at first, but for people who select Java code (e.g. an NLS constant) or a string literal, we create more work (deleting the quotes is harder than adding them). Some other trivial changes: - make sure the cursor is at the end of the template and not inside the listener - used our copyright notice format - updated copyright date Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=cf4a6483b2973f177218ad73eb61797eee47e0e5
NOTE: Change is too small for N&N.
> Using quotes looks helpful at first, but for people who select Java code > (e.g. an NLS constant) or a string literal, we create more work (deleting the > quotes is harder than adding them). I think selecting a string and using a template on this, is the exception not the normal case. Leaving that change out, still creates syntax errors for the IMHO 90% use case. I assume you, the JDT developers, are also good example for the usage of the SWT templates. Is your normal flow to select a text or to use the templates without text selection? Please close the bug again, if your experience is different from mine.
(In reply to comment #20) > > Using quotes looks helpful at first, but for people who select Java code > > (e.g. an NLS constant) or a string literal, we create more work (deleting the > > quotes is harder than adding them). > > I think selecting a string and using a template on this, is the exception > not the normal case. Correct. But, users can easily select code, like <MyString.getText()>. And we also don't add quotes around the word selection in other templates, e.g. 'sysout'.
If there a construct to detect user selection? If, we could have a conditional statement.
(In reply to comment #22) > If there a construct to detect user selection? If, we could have a > conditional statement. Not sure what you exactly mean.
I don't now if the templates allow to check if the word selection is empty. If this would be possible we could create templates which are both correct in case something is selected (use no "") and if case nothing is selected (use "" to avoid syntax error). In ;seudo-code: if word selection.isEmpty()) { use quotes for the template} else { no quotes for the template}
(In reply to comment #24) > I don't now if the templates allow to check if the word selection is empty. > If this would be possible we could create templates which are both correct > in case something is selected (use no "") and if case nothing is selected > (use "" to avoid syntax error). That would not help much. I can navigate to setText(|) using Tabs and then start to add code using content assist. I'd hate if there are "" at this location in this scenario. Also, for me, having setText("") is worse than having a compile error that indicates, work has to be done there. And to answer your question: there is no way to interpret the word selection inside the template definition.
