Community
Participate
Working Groups
Created attachment 145268 [details] ConsoleViewWordWrapPatch the attached patch add Word Wrap action to Console View
Created attachment 145269 [details] word wrap screenshot added screenshot
Any news regarding this *old* issue?
Duplicate. *** This bug has been marked as a duplicate of bug 35745 ***
I like it, why has it never considered and marked as duplicate of bug 35745 when nobody never worked on bug 35745?
(In reply to comment #4) > I like it, why has it never considered and marked as duplicate of bug 35745 > when nobody never worked on bug 35745? I can't speak for why it was "never considered", but as to your duplication question, this process is the norm for projects hosted at Eclipse.org. Both bugs are talking about the same issue and so the one created later was marked as a duplicate of the earlier one.
Makes sense but then I hope someone will one day considers the effort done. I makes me feel like it's wasted...
(In reply to comment #6) > Makes sense but then I hope someone will one day considers the effort done. I > makes me feel like it's wasted... I am not sure I understand what you are saying here. What is "wasted"?
It seems to me that nobody never tried to review and test the attached patch. Since it's already almost 3 years the work done by Gaetano Santoro seems wasted. That's what I meant. Also the fact that this bug report is marked as duplicate makes me wonder if the committer/s assigned to the referenced bug is/are aware of the existence of this patch...
(In reply to comment #8) > It seems to me that nobody never tried to review and test the attached patch. > Since it's already almost 3 years the work done by Gaetano Santoro seems > wasted. That's what I meant. Ah, I see. My mistake. I did not even realize there was a patch to this bug since I was only reading the newest comments.
I will reopen this bug. When cleaning up the duplicate reports we must have not seen the patch (so the oldest report is kept). I don't know if this is something we have time to review for 4.3.
*** Bug 35745 has been marked as a duplicate of this bug. ***
Can anyone tell me how to apply the patch? thank you in advance.
(In reply to comment #12) > Can anyone tell me how to apply the patch? thank you in advance. Copy the patch to your clipboard or save it locally. Right click on a project, apply patch...
Nice patch. Still works! Rebased onto Luna and added to https://git.eclipse.org/r/20776 code review. Done as part of the https://wiki.eclipse.org/Hackathon_Hamburg_2013 I added Gaetano Santoro as author in the copyright header as the is less than 250 lines and was contributed before we had the CLA.
Gaetano can you sign the CLA, see http://www.eclipse.org/legal/clafaq.php as you are the original author and I am just the patch committer. This is currently holding of the review process.
signed Thanks Gaetano
I am still getting request rejected "Authored by: Gaetano Santoro error: The author does not have a Gerrit account. All authors must either be a commiter on the project, or have a current CLA on file. Please see http://wiki.eclipse.org/CLA" This may be a timing/cache issue. However just to make sure: can you login to http://git.eclipse.org/c/ to create a Gerrit account? You will need this anyway to submit patches directly in the future.
(In reply to Matthias Mailänder from comment #17) > I am still getting request rejected The gerrit link: https://git.eclipse.org/r/#/c/20776/ Review comments: 1. The new action does not need to be API - should be moved to the org.eclipse.ui.internal.console package. 2. I would prefer the action to be a toolbar action so users get reminded that they have the setting turned on. 3. It would be better to use the new menus extension point for contributing a command / handler / toolbar button
(In reply to Michael Rennie from comment #18) > 2. I would prefer the action to be a toolbar action so users get reminded > that they have the setting turned on. Can you suggest an icon we should use? > 3. It would be better to use the new menus extension point for contributing > a command / handler / toolbar button I think all the other contributions are based on the old one, I think for consistency this new contribution should use the same approach.
(In reply to Lars Vogel from comment #19) > (In reply to Michael Rennie from comment #18) > > > 2. I would prefer the action to be a toolbar action so users get reminded > > that they have the setting turned on. > > Can you suggest an icon we should use? > How about this one: platform:/plugin/org.eclipse.ui.workbench.texteditor/icons/full/etool16/block_selection_mode.gif > > 3. It would be better to use the new menus extension point for contributing > > a command / handler / toolbar button > > I think all the other contributions are based on the old one, I think for > consistency this new contribution should use the same approach. The goal is to remove all of the deprecated contributions, so adding more of them does help that end. See bug 390869.
(In reply to Michael Rennie from comment #20) Updated Gerrit review to address 1.) and 2.). I had to change the author tag in the commit, as I got the message that I'm not allowed to push as non committer on behave of others. > The goal is to remove all of the deprecated contributions, so adding more of > them does help that end. See bug 390869. I didn't find the time to do this. Maybe Gaetano or someone else can help here?
(In reply to Michael Rennie from comment #18) > (In reply to Matthias Mailänder from comment #17) > > I am still getting request rejected > > The gerrit link: https://git.eclipse.org/r/#/c/20776/ > > Review comments: Mike, it would make sense to add review comments that apply to a concrete Gerrit change directly in Gerrit, or at least add a comment in Gerrit that you made comments in the bug report. I was confused to see new patch sets coming for the change in Gerrit without any explanation/reason.
(In reply to Michael Rennie from comment #20) > > I think all the other contributions are based on the old one, I think for > > consistency this new contribution should use the same approach. > > The goal is to remove all of the deprecated contributions, so adding more of > them does help that end. See bug 390869. I disagree. We should stay consistent with what we have. That makes the code easier to understand. Plus, I don't see us doing bug 390869 - we have more important/interesting work to do.
(In reply to Lars Vogel from comment #21) > (In reply to Michael Rennie from comment #20) > > Updated Gerrit review to address 1.) and 2.). I had to change the author tag > in the commit, as I got the message that I'm not allowed to push as non > committer on behave of others. > > > The goal is to remove all of the deprecated contributions, so adding more of > > them does help that end. See bug 390869. > > I didn't find the time to do this. Maybe Gaetano or someone else can help > here? Hi All, at the moment I'm a little busy with test/validation, so I can do this in the coming months. However, if there are many contributions using the deprecated approch, having this one converted don't change much. If it is not urgent I will do this in the coming months.
(In reply to Dani Megert from comment #23) > (In reply to Michael Rennie from comment #20) > > > I think all the other contributions are based on the old one, I think for > > > consistency this new contribution should use the same approach. > > > > The goal is to remove all of the deprecated contributions, so adding more of > > them does help that end. See bug 390869. > > I disagree. We should stay consistent with what we have. That makes the code > easier to understand. Plus, I don't see us doing bug 390869 - we have more > important/interesting work to do. If that is the case, I think the contribution is ready for review. I move the menu to the toolbar and added an icon as Michael suggested.
(In reply to Michael Rennie from comment #20) > (In reply to Lars Vogel from comment #19) > > (In reply to Michael Rennie from comment #18) > > > > > 2. I would prefer the action to be a toolbar action so users get reminded > > > that they have the setting turned on. > > > > Can you suggest an icon we should use? > > > > How about this one: > platform:/plugin/org.eclipse.ui.workbench.texteditor/icons/full/etool16/ > block_selection_mode.gif That's a no go. We should not use the same icon for two different things. Especially, not when both are showing text and allow selection.
I created a wordwrap icon https://git.eclipse.org/r/#/c/21787/
(In reply to Matthias Mailänder from comment #27) > I created a wordwrap icon https://git.eclipse.org/r/#/c/21787/ Can you also provide the GIF? We might switch to SVG at some point, but currently all icons are GIFs in ui.console.
(In reply to Dani Megert from comment #28) > (In reply to Matthias Mailänder from comment #27) > > I created a wordwrap icon https://git.eclipse.org/r/#/c/21787/ > > Can you also provide the GIF? We might switch to SVG at some point, but > currently all icons are GIFs in ui.console. If you are OK with the icon, I create a png file for it via the SVG to png generator.
(In reply to Lars Vogel from comment #29) > (In reply to Dani Megert from comment #28) > > (In reply to Matthias Mailänder from comment #27) > > > I created a wordwrap icon https://git.eclipse.org/r/#/c/21787/ > > > > Can you also provide the GIF? We might switch to SVG at some point, but > > currently all icons are GIFs in ui.console. > > If you are OK with the icon, I create a png file for it via the SVG to png > generator. Yes please. I think it could work, but need to see it live for a final "go".
Created attachment 239961 [details] word wrap icon preview in PNG format See https://git.eclipse.org/r/#/c/21787/ for the source SVG.
(In reply to Matthias Mailänder from comment #31) > Created attachment 239961 [details] > word wrap icon preview in PNG format > > See https://git.eclipse.org/r/#/c/21787/ for the source SVG. Thanks. Icon added to org.eclipse.ui.images with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5278f7b48ece0fb02947eb6ffc47d1fa50fa1917
https://git.eclipse.org/r/#/c/20776/ has been updated and awaits feedback.
Created attachment 240228 [details] screenshot of the latest revision Looks better with a button. Thanks Lars.
(In reply to Matthias Mailänder from comment #34) > Created attachment 240228 [details] > screenshot of the latest revision > > Looks better with a button. Thanks Lars. Indeed, but as said before, we need them as GIFs.
(In reply to Dani Megert from comment #35) > (In reply to Matthias Mailänder from comment #34) > > Created attachment 240228 [details] > > screenshot of the latest revision > > > > Looks better with a button. Thanks Lars. > > Indeed, but as said before, we need them as GIFs. PNG should be OK, right? We anyhow want to migrate to PNG.
(In reply to Lars Vogel from comment #36) > (In reply to Dani Megert from comment #35) > > (In reply to Matthias Mailänder from comment #34) > > > Created attachment 240228 [details] > > > screenshot of the latest revision > > > > > > Looks better with a button. Thanks Lars. > > > > Indeed, but as said before, we need them as GIFs. > > PNG should be OK, right? We anyhow want to migrate to PNG. It will be OK, but having a mix is not good. So, you can either wait with this bug until converted to PNG or deliver the GIF. Probably the former, given there's still work to do after Mike's review.
What is holding this up? Do we need to convert more icons from GIF to PNG?
(In reply to Matthias Mailänder from comment #38) > What is holding this up? Do we need to convert more icons from GIF to PNG? The enablement / visibility / preference state of the action does not work. See my review comments on patch set 6.
Created attachment 241414 [details] Word Wrap is now part of output group, right next to scroll lock Okay I rewrote it in patch set 7 to match the way the other buttons (took scroll lock as a template) are integrated. This also fixes the problem that a non-functional button is shown on empty consoles.
https://git.eclipse.org/r/#/c/20776/
(In reply to Matthias Mailänder from comment #40) > Created attachment 241414 [details] > Word Wrap is now part of output group, right next to scroll lock > > Okay I rewrote it in patch set 7 to match the way the other buttons (took > scroll lock as a template) are integrated. This also fixes the problem that > a non-functional button is shown on empty consoles. Looks much better now. There is however one major remaining issue (and few small ones listed below): the console does not wrap output for successive consoles automatically. Steps: 1. create an IOConsole and check on the word wrap option. If there is any overly long output in the console it will be wrapped (GOOD) 2. run the following snippet: package console.test; public class FloodConsole { public static void main(String[] args) { for(int i = 0; i < 100000; i++) { System.out.println("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); //$NON-NLS-1$ } } } notice that the word wrap action is still checked on (maintained its state - GOOD) but that the output is not wrapped (BAD) 3. If you toggle the action the output from the above snippet will be wrapped I took a quick peak at the code and there are few additional things: 1. IOConsolePage#createActions, is missing a call to #setWordWrap 2. ConsoleView#showPageRec, is missing a call to IOConsolePage#setWordWrap (at the end of the method) 3. The state set in IOConsoleViewer is not used 4. IOConsoleViewer#isWordWrap is not used
Fixed the inconsistencies in https://git.eclipse.org/r/#/c/20776/7..8/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/ConsoleView.java,unified and following, but I think it still does not memorize the previous word wrap setting. Read through the console code, but I am not sure where I am missing a hook.
(In reply to Matthias Mailänder from comment #43) > Fixed the inconsistencies in > https://git.eclipse.org/r/#/c/20776/7..8/org.eclipse.ui.console/src/org/ > eclipse/ui/internal/console/ConsoleView.java,unified and following, but I > think it still does not memorize the previous word wrap setting. Read > through the console code, but I am not sure where I am missing a hook. Looks like the successive console hook you are missing is in IOConsoleViewer#setWordWrap, you have to add the following: getTextWidget().setWordWrap(wordwrap); Also, I found that some places you invert the value of word wrap from the checked state of the action, but you don't need to - I guess when you looked at the scroll lock example you just copied the same behavior? In the scroll lock case, if the action is checked autoscrolling is false, in the wordwrap case if the action is checked wrapping is true. in ConsoleView: - ((IOConsolePage)page).setWordWrap(!fWordWrap); + ((IOConsolePage) page).setWordWrap(fWordWrap); in IOConsolePage: - fWordWrapAction.setChecked(!wordwrap); + fWordWrapAction.setChecked(wordwrap); - setWordWrap(!fWordWrapAction.isChecked()); + setWordWrap(fWordWrapAction.isChecked()); in IOConsoleViewer - private boolean fWordWrap = true; + private boolean fWordWrap = false;
Thanks for the help. I definitely lost an overview on the inverted scroll lock and the modular approach over so many files. Updated https://git.eclipse.org/r/#/c/20776/
(In reply to Matthias Mailänder from comment #45) > Thanks for the help. I definitely lost an overview on the inverted scroll > lock and the modular approach over so many files. Updated > https://git.eclipse.org/r/#/c/20776/ Merged with: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=2fd676ca07dc7f2cf5d9f0e74d312b15e665b067 Updated bundle versions / copyright headers, etc with: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=a706cd47189727205482922c03b0bc31da7680a7
Verified using Eclipse SDK Version: Mars (4.5) Build id: I20140806-2000
If I check the "Word Wrap" and restart eclipse, it's gone. Has someone considered to introduce this as permanent setting in Preferences -> Run/Debug -> Console? Or just simpler, serialize the flag in the workspace to ws\.metadata\.plugins\org.eclipse.core.runtime\.settings\org.eclipse.debug.ui.prefs Would this make sense? For me it's annoying that flag needs to be set each time I start eclipse :(
(In reply to Konstantin F. from comment #48) > If I check the "Word Wrap" and restart eclipse, it's gone. Has someone > considered to introduce this as permanent setting in Preferences -> > Run/Debug -> Console? Or just simpler, serialize the flag in the workspace > to > ws\.metadata\.plugins\org.eclipse.core.runtime\.settings\org.eclipse.debug. > ui.prefs > > Would this make sense? For me it's annoying that flag needs to be set each > time I start eclipse :( Please create an enhancement which we can consider for 4.7 based on other inputs.
I've created a feature request: 491853
