This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Option to remove whitespace at end of line | ||
---|---|---|---|
Product: | editor | Reporter: | Scott Ganyo <sganyo> |
Component: | Formatting & Indentation | Assignee: | Miloslav Metelka <mmetelka> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | abadea, dodger, dprusa, dynamite, geertjan, gsporar, jglick, joshis, mkrauskopf, pjiricka, rstrobl, tboudreau, ttran, vnicolici |
Priority: | P2 | ||
Version: | 3.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | 137529 | ||
Bug Blocks: | 133913 |
Description
Scott Ganyo
2001-06-21 14:58:01 UTC
Target milestone -> 3.3.1. Set target milestone to TBD Set target milestone to TBD We have programmers using SunOne Studio or JBuilder and JBuilder allows removing trailing whitespaces to save space but cvs file comparison shows more changed in the file because it was previously edited with SunOne Studio. It makes file sizes smaller too. *** Issue 19306 has been marked as a duplicate of this issue. *** *** Issue 23112 has been marked as a duplicate of this issue. *** *** Issue 32491 has been marked as a duplicate of this issue. *** *** Issue 40669 has been marked as a duplicate of this issue. *** Solving this issue should be a trivial task for your developers, I would like to have this feature in NB as soon as possible. See other suggestions in issue #59884. *** Issue 59884 has been marked as a duplicate of this issue. *** Note there is a module, contrib/stripwhitespace that adds a function to the Edit menu to strip trailing whitespace. In a perfect world, probably it should be more automatic than that (on save? - but there are file types where you may *want* trailing whitespace...). Anyway, it should show up on nbextras.org at some point, or can be built from source in the meantime. I've been using that module. It's nice, but it'd be nicer if it had hotkeys :) The tasklist/checkstyle also provides this feature. [ If you checkstyle_checks.xml lists trailing spaces as a checkstyle violation. ] This module then adds each line with trailing space to the suggestions list, from there they can all be automatically fixed. It also works across the whole project via Suggestions "Scan directory for Suggestions". Many issues have been marked as duplicates of this one that really are asking for solutions to different problems, or at least to this *and* other problems. 19306 and 23112, both not only wnat the option for the whitespace to disappear on save, they also want the 'end' key to behave differently in the editor when the whitespace hasn't yet been stripped. 32491 59884 is more a duplicate of my issue 23315 (that isn't even mentioned here.) Where the indentation engine inserts indenting spaces when 'enter' is hit. If enter was hit prematurely, you now have to hit 'backspace' 4,8,12, or more times to get back up to the line you were on. Another issue recommends making whitespace(trailing only?) visible in the editor. I see three issues: 1. Allowing user to Configure NB to remove whitespace on save. 2. Allowing user to Configure the indentation engine, 'enter', and 'backspace' to work well together, and not add extra whitespace to begin with. 3. Allowing user to request that the editor show whitespace. #1 may be solved by the plugin Tim mentioned. #2 and #3 seem to still be an open issue, and I would contend that some of these duplicates aren't duplicates of this issue at all. Unfortunately #2 will probalby have to wait for the indentation engine re-write that's been on the NB todo list for what seems like forever. I believe the indentation engine rewrite stuff will probably happen for NetBeans 6 - some kind of unified handling of formatting that does the right thing for both newly generated code and can be manually invoked over a file. Re the highlighting of whitespace, you *can* set the editor to use a different color for *all* whitespace, which is probably undesirable if you just want to see trailing whitespace. It would also be possible to use DrawLayer support in the editor to do the same thing the mark occurances module does. Probably you'd want some hotkey that makes it visible and then hides it again - it would personally drive me nuts to have it on all the time, as if I'm fixing a bug in a file in CVS that has existing trailing whitespace, I'm not going to delete it because it would pollute the diff of my fix - nobody would be able to tell real changes from formatting changes in the future if they needed to look up what changed when I did the fix. I'm working on adding trailing whitespace highlighting to the contrib/stripwhitespace module. It already kind of works, I just need to find some time to finish and commit it. I commited the support for highlighting trailing whitespace to contrib/stripwhitespace. Hope it solves #3 at least for someone. There is an experimental functionality in trunk that removes ending whitespace on previously edited line. If there is an ending whitespace added (either by user or by indentation engine) to a particular line and the user starts to edit another line then the ending whitespace on the previously edited line gets removed automatically. Unfortunately there are several complaints regarding this (e.g. issue 74149) so we would like to modify it to only remove whitespace before save operation on the lines that were modified (to not cause extra VCS diffs). FWIW, anyone who wants the StripWhitespace module, it's available from http://nbextras.org - and Andrea did a wonderful job adding the ability to highlight all trailing whitespace to it. Thanks! BTW that's "Andrei", not "Andrea", and no, I'm not Italian :-) Isn't this issue now resolved with the action: Source -> Remove Trailing Spaces ? There are still improvements to be considered, especially the automatic end WS removal upon save operation. Now it's clear that the original assumption to remove WS "aggressively" upon leaving a line (e.g. like Vim does) that was tested in the trunk some time ago was not accepted by our users so the WS needs to be removed explicitly and best upon save. It should handle just the modified lines so that it does not produce extra diffs (of course unless the user really wants to do it for the whole source). Setting TM to future as I'm not sure whether we will be able to do anything more regarding this for 6.0. *** Issue 117968 has been marked as a duplicate of this issue. *** Given that we have local history and so forth, and actually know what lines have been changed, I would love to see an option to strip trailing spaces *only from modified lines*. That way I don't create gratuitous diffs, but I also don't contribute to the problem any further. I don't think there is even any need for the local history module. The editor module just needs to keep track of which lines have been modified (or inserted) in the document since it was last opened or saved; and upon saving, strip WS from them all. This is one of the things that I would like to finally implement in NB7. Given that the extra WS affects all the sources I consider this is an important feature. Current implementation only removes WS on lines where typing occurred to avoid extensive changes in version-control systems. I hope that this approach should be the most beneficial and thus I did not implement any visual option to disable this feature yet. The feature adds a little SPI on org.openide.text side (a "beforeSaveRunnable" document property) which I have properly documented in arch and api changes docs and added a test for it. There are no API classes introduced. changeset: 84586:a12f34869bee tag: tip parent: 82742:f6b2e0181e07 user: Miloslav Metelka <mmetelka@netbeans.org> date: Mon Jun 16 14:28:00 2008 +0200 summary: #13063 - Remove whitespace at end of line. http://hg.netbeans.org/main/rev/a12f34869bee You broke http://deadlock.netbeans.org/hudson/job/validate-project-xmls/332/testReport/org.netbeans.nbbuild/ValidateAllBySchema/editor_lib_nbproject_project_xml/ editor.lib/nbproject/project.xml:133: cvc-complex-type.2.4.d: Invalid content was found starting with element 'compile-dependency'. No child element is expected at this point. If the caret is on a line ending with space when you save, this whitespace is not removed, even if you had edited that line during this session, and even if the caret was not at the end of the line. Intentional? Also if you only delete characters on a line, or cut or copy segments in the line - i.e. make any modifications other than typing new characters - whitespace will not be deleted from the end of it. 1. was a fix of issue 137529. I tried to convince Martin Adamek that it's better to remove the WS on current line too but he insisted that it's a bug. 2. Regarding removals I've only made a note in TrailingWhitespaceRemove.removeUpdate() so far: // Currently do not handle in any special way but // Since there's a mod on the line there will be a diff // so it should not matter much if the ending WS is removed too. I will implement it. IMHO it should only suffice to create empty mod regions and process the empty regions too. Hi, one little thing - which is not very important and is related to 1) from the previous post - can be enhanced (I am sorry if it was briefly mentioned somewhere). I understood that WS are not currently removed from the line with a caret at all, which is not problem for me. But should they never be removed from this line, as it happens now (200807181543)? Example: 1. Suppose you have (<EOF>=EOF, <\n>=NL, |=caret) public class Class { |<\n> int i = 0;<\n> }<EOF> 2. Press Ctrl+S -> WS in the 1st line are (correctly) preserved 3. Put some WS to the end of the second line, like this public class Class { <\n> int i = 0; |<\n> }<EOF> 4. Press Ctrl+S I would expect that the result is (no WS on the first line): public class Class {<\n> int i = 0; |<\n> }<EOF> But it is not (WS on the first line remain), result looks like this: public class Class { <\n> int i = 0; |<\n> }<EOF> The line with the caret should still be marked as modified after Ctrl+S, if it has been modified before. That way the trailing WS will get removed next time you save the file (if you move the caret elsewhere). Obviously, even this will not work when you close/open the file between the two Ctrl+S'. Could you please file a separate P3 defect for this? Thanks OK, I will just copy&paste a part of my previous post... joshis' example is a feature as far as I am concerned. Once in a while you might have a good reason to explicitly put a space at the end of a line. Rare, but it happens. If you want to do this now in 6.5, the only way is to save immediately after typing the space. If that space were later removed when you made unrelated edits to the file, you would be fighting against the IDE. So I think the current behavior is correct - reset the list of edited lines at each save. jglick - you are right about the fact it would be impossible to put a whitespace in the end of the line. Though I can't figure out an example of any rare situation where you need a whitespace in the end of the line (for example if you edit properties file, you usually put a '\' escaping char as the last char on the line), I believe there might be some. But we can discuss to what extent the way it (adding WS to the end of line) works now is friendly. What I mean: does it make some sense that if you want to place a WS in the end of line, you have to save the file while caret is on that line? What if you want to do this on 2 lines? Should you need two save operations for that? This seems to me like a pretty much strange/hacky way to do that (unless user knows it, he won't use it as he cannot figure it out). IMO it is better to turn the feature off if you need WS in the end of the line - much more systematic solution. Maybe the best would be allow user to turn "Remove WS on the blank line" off and give him a chance to remove WS using some separate action... In any case, I think that in spite of a good point made by jglick the current implementation is not correct... Removal of all whitespaces in the end of the line was the reason why this was implemented, I believe. joshis: BTW regarding an explicit WS removal there is an action Edit->Remove Trailing Whitespace. A note that I have discussed this in issue 137529 when I was trying to justify - unsuccessfully ;) an "aggressive" WS removal: Let's treat your example as two (unrelated) bug-fix edits of a particular source. Let's assume the source is stored in a VCS (first check-in after step 2 and another check-in after step 4) and that each bug-fix modifies a different part of the source. What I don't like here is that the second commit may contain a WS removal that is logically related to bug-fix 1. > BTW regarding an explicit WS removal there is an action Edit->Remove Trailing Whitespace. Very good, I didn't know about this. > A note that I have discussed this in issue 137529 when I was trying to justify - unsuccessfully ;) an "aggressive" WS removal... I know - I have read it:)... I see where the problem might be, but IMO it is not too big deal. You will have a few WS (just a few WS in the end of only one line) removed and it will logically belong to the previous commit - is that a real problem for someone who wants to check revisions? I would suggest a "fake aggressive" approach - when user saves file: 1. WS's are removed on EVERY line, containing the line with a cursor 2. Caret does not move (it will potentially remain "after EOL"), as if there were some "fake WS's" 3. If user goes to another line (bellow or above), "fake WS's" will be removed on the "previously current" line 4. If user starts typing, appropriate number of spaces will be inserted instead of "fake WS's" I know this would be a bit harder to implement, but it can work quite well in the end... But it is not necessary - currently it works quite fine, I can invoke "Remove Trailing Whitespace" if I desperately needed to remove all WS (this even cannot be called a "workaround")... joshis, I think mmetelka mentioned the same idea in bug 137529: "We could support "virtual caret position" in the future (which would look like the caret is at a particular column but there would be no corresponding text underneath) but it's not very straightforward to implement such thing with the Swing text package APIs." I think it would work quite well, although is it worth all the trouble if you can just hit the tab key once after saving, if you happen to be on an indented line? Leaving 'real' whitespace on the current line is incorrect in my opinion. > joshis, I think mmetelka mentioned the same idea in bug 137529: ...
Oh... I am sorry - this is quite embarrassing... I missed this somehow or I didn't read the discussion after the post
was made - I don't like repeating someone's ideas:-/...
I'd like the option to turn this feature off, I work in an environment where other developers aren't using NetBeans and their editors don't remove whitespace, so when NetBeans removes the whitespace it produces a lot of spurious diffs in the version control system. johnsi: The option will be added however there should be no spurious diffs unless you've hit an issue 138951 (document reload marks all lines as modified) which should be fixed already. Could you eventually file an issue describing your usecase producing spurious diffs? Thanks. I have realised one unpleasant side effect of the current WS removal implementation. Since the Save operation physically removes extra whitespace characters from the document it may discard subsequent edits present in undo queue (by creating new edits for WS removal). Example: 1) do several edits (e.g. 5) to document. Save. 2) Undo last 2 edits. Save. 3) Redo will do nothing since the Save has produced an undo-edit that discarded rest of undo queue. Side note: the redo problem existed even before WS removal due to issue 21237 but until WS removal it was fixable. With WS removal the situation would be bad. It came to my mind that we could possibly keep the WS in the document but let the EditorKit.write() operation to only write the data without the trailing WS. The accounting of modified regions would stay the same. I like the fact that besides getting rid of the Redo-after-save problem we would also eliminate WS-must-be-retained-on-line-with-caret problem in an elegant way. As usually there may be problems with the proposed approach since the content of document and file on disk may differ. In fact they may differ even now due to possible '\n' => '\r\n' translation but that's a predictable way that the tools may simulate. If everyone would follow a policy that if there's an opened document for a file then all the data exchange would go through the document and vice versa then all should be fine. However when talking to java guys there's a possibility that: 1. there's an opened document. 2. modifications are done and doc is saved. (file content may now differ from the one in the doc) 3. some tools find out that the doc is unmodified and so they prefer to use the file's content and use integer position. This would not work correctly unless either working with the document if opened or using line:column references. In summary I like the idea of kit.write() but I understand this may be a long way to do the necessary changes. *** Issue 72528 has been marked as a duplicate of this issue. *** A slight improvement (as requested by Jesse) - WS on line containing caret now gets removed if it's behind the caret. http://hg.netbeans.org/main/rev/12a049e7fa0f I have been noticing that this feature does not work reliably. I often create a new file and do a bunch of different things and finally save, and notice that several lines in the file still have trailing whitespace. I am not yet sure how to reproduce but if I figure it out of course I will file a bug blocking this one. Integrated into 'main-golden', available in build *200808280201* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/12a049e7fa0f User: Miloslav Metelka <mmetelka@netbeans.org> Log: #13063 - Trailing whitespace removal - WS on line containing caret gets removed if it's behind the caret. *** Issue 148726 has been marked as a duplicate of this issue. *** Whether or not to remove white space should be an option. I recently switched from NetBeans 7.1 to 7.2 and 7.2 removes "trailing whitespace" in blank lines when you highlight code and then do Tab or Shift+Tab (to change the indentation of the selected lines). I personally prefer the way it was done in 7.1 where even the blank lines were indented with the rest of the code. There's a configurable option in Tools->Options->Editor->General->Remove Trailing Whitespace. This doesn't work. I just configured it to that, but after saving the file, it has white spaces at the end of the file again. This is fatal because you don't even notice it. And even if you noticed it and you have activated the option to show white spaces you can remove them indeed but after saving... (continue reading at the beginning ;)). I can confirm that the current option in Tools->Options->Editor->General->Remove Trailing Whitespace does not do anything, as reported by mwitzmann@netbeans.org. I am using Product Version: NetBeans IDE 7.4 (Build 201310111528) Updates: NetBeans IDE is updated to version NetBeans 7.4 Patch 2 Java: 1.7.0_25; OpenJDK 64-Bit Server VM 23.7-b01 Runtime: OpenJDK Runtime Environment 1.7.0_25-b30 System: Linux version 3.8.0-35-generic running on amd64; UTF-8; en_GB (nb) I have the above setting set to "All Lines" for "All Languages". I have the "Reformat" setting set to "None" for "All Languages". (If I set "Reformat" to "All Lines", then it does seem to also remove whitespace. However, I just want to remove whitespace, not reformat.) Marking re-opened, per above comments. Will be possible to add option that move cursor to begin of line when he is behind some whitespace characters? (and then remove them too) Its annoying to remember before save move cursor or delete whitespace characters in current line. Its happen very often because cursor is always indent to rest of current block of code. Fixing cursor position after that is faster than removing whitespace characters (one <tab> vs usually 2x<backspace>+<tab> or <up>+<ctr+s>+<down>+<tab> to go back to previous cursor position). As this will be new options (I suggest new checkbox for that) nobody who like or depend on current behavior will be affected by this. This will made bug 137529 a optional feature :) As reopened, this is a defect, not an enhancement. I can't reproduce the problem on recent development builds when "Reformat" is set to "None" for "All Languages" and "Remove Trailing Whitespace From" is set to either "All Lines" or "Modified Lines Only". Please note that the setting is now on Editor->On Save tab. If it still would not work for you please reopen the issue (please double check first that the setting for the particular file type is not overriden or it's overriden to the desired setting). (In reply to Yankes from comment #55) > Will be possible to add option that move cursor to begin of line when he is > behind some whitespace characters? (and then remove them too) This was an initial setup originally but later it was decided that retaining of caret position is more important that WS removal. Anyway I've added a non-visual startup parameter (possibly see http://wiki.netbeans.org/FaqStartupParameters) that can be used to remove the WS even on current line: -J-Dorg.netbeans.editor.remove.whitespace.on.current.line=true http://hg.netbeans.org/jet-main/rev/3d5857937700 Integrated into 'main-silver', will be available in build *201407170001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/3d5857937700 User: Miloslav Metelka <mmetelka@netbeans.org> Log: #13063 - Option to remove whitespace at end of line - added non-visual option for WS removal on current line. (In reply to Miloslav Metelka from comment #57) > (In reply to Yankes from comment #55) > > Will be possible to add option that move cursor to begin of line when he is > > behind some whitespace characters? (and then remove them too) > This was an initial setup originally but later it was decided that retaining > of caret position is more important that WS removal. > Anyway I've added a non-visual startup parameter (possibly see > http://wiki.netbeans.org/FaqStartupParameters) that can be used to remove > the WS even on current line: > > -J-Dorg.netbeans.editor.remove.whitespace.on.current.line=true > > http://hg.netbeans.org/jet-main/rev/3d5857937700 I'm very thankful for that feature! Work exactly like I want. |