Bug 228495 - XML formatter removes empty lines
Summary: XML formatter removes empty lines
Status: VERIFIED FIXED
Alias: None
Product: WTP Source Editing
Classification: WebTools
Component: wst.xml (show other bugs)
Version: 3.0   Edit
Hardware: PC Linux
: P3 normal with 4 votes (vote)
Target Milestone: 3.0.1   Edit
Assignee: Nick Sandonato CLA
QA Contact: Nitin Dahyabhai CLA
URL:
Whiteboard:
Keywords: contributed
: 239678 241025 247042 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-04-23 15:11 EDT by Andreas Schildbach CLA
Modified: 2008-09-11 14:47 EDT (History)
10 users (show)

See Also:
for.work.things: review+
thatnitind: review+

Attachments
patch (9.78 KB, patch)
2008-04-25 15:29 EDT, Nick Sandonato CLA 		no flags Details | Diff
patch for xml content (17.55 KB, patch)
2008-06-24 17:17 EDT, Nick Sandonato CLA 		no flags Details | Diff
patch to keep from dirtying unchanged regions (18.55 KB, patch)
2008-07-02 15:03 EDT, Nick Sandonato CLA 		no flags Details | Diff
patch to keep from dirtying unchanged regions (17.89 KB, patch)
2008-07-02 16:46 EDT, Nick Sandonato CLA 		no flags Details | Diff
third time's a charm (17.31 KB, patch)
2008-07-02 16:56 EDT, Nick Sandonato CLA 		bjorn.freeman-benson: iplog+
Details | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schildbach CLA 2008-04-23 15:11:25 EDT 
Build ID: I20080330-1350

Steps To Reproduce:
XML formatter removes empty lines, and there is no way to prevent it.

This has not been a problem on Eclipse 3.3.

More information:
Ubuntu Gutsy, Java 1.5.0_13-b05
Comment 1 Nick Sandonato CLA 2008-04-25 15:29:37 EDT 
Created attachment 97672 [details]
patch

This patch should address the XML formatter not keeping blank lines.

Also added a junit test case to test keeping blank lines, as well as fixed up some preferences for some of the old test cases that expected blank lines would be cleared, when by default, they are not.
Comment 2 Amy Wu CLA 2008-04-30 19:31:46 EDT 
The patch fixes empty lines between tags, but if you have empty lines in tag content/within a start & end tag, then they will always be removed.
Comment 3 Nitin Dahyabhai CLA 2008-05-29 14:39:35 EDT 
Bumping to 3.0.1 pending addressing of issue from comment 2.
Comment 4 Nick Sandonato CLA 2008-06-24 17:17:26 EDT 
Created attachment 105757 [details]
patch for xml content

This patch should address keeping blank lines in XML content as it was in previous versions of WTP. 

It will also fix a problem where the blank lines around comments were always being kept, and comments were not properly indented when on their own line.
Comment 5 Amy Wu CLA 2008-06-30 17:17:01 EDT 
The patch looks pretty good, but I found a case where formatting an already formatted file will mark the file as dirty.

1. create an xml file with the following:
<tag>


content

</tag>
2. format and save
3. format again
I expected the file to not be dirtied since it was already formatted, but that was not the case.

I believe there needs to be some logic added in the collapseAndIndent method to check that if the existing whitespace is already going to be the same as what you were going to replace the whitespace with, then there's no need to replace.

Other than that, patch looks good. Especially with unit tests.

If you want, I can change my review to a + and get this committed for 3.0.1 and then open a new bug on this one last issue.  Or we can keep this bug open while you work on the last issue.  I'm fine with either way.
Comment 6 Nick Sandonato CLA 2008-07-02 15:03:48 EDT 
Created attachment 106343 [details]
patch to keep from dirtying unchanged regions

Added logic to collapseAndIndent so that if the correct indenting is equivalent to the existing whitespace, no change is made.
Comment 7 Nick Sandonato CLA 2008-07-02 16:46:31 EDT 
Created attachment 106357 [details]
patch to keep from dirtying unchanged regions

The previous patch had an extra test file in. Removed it.
Comment 8 Nick Sandonato CLA 2008-07-02 16:56:59 EDT 
Created attachment 106359 [details]
third time's a charm

Sorry left a testing System.out in there.
Comment 9 MarkSinke CLA 2008-07-05 17:17:51 EDT 
*** Bug 239678 has been marked as a duplicate of this bug. ***
Comment 10 Amy Wu CLA 2008-07-09 16:07:07 EDT 
Okay, fix looks good.
Comment 11 Nick Sandonato CLA 2008-07-09 16:26:44 EDT 
Adding Nitin for review for 3.0.1 patches.
Comment 12 Nitin Dahyabhai CLA 2008-07-15 23:25:11 EDT 
*** Bug 241025 has been marked as a duplicate of this bug. ***
Comment 13 Nitin Dahyabhai CLA 2008-07-16 04:38:55 EDT 
released
Comment 14 Nick Sandonato CLA 2008-07-25 10:17:16 EDT 
Verified in M-3.0.1-20080724062240.
Comment 15 Amy Wu CLA 2008-09-11 14:47:20 EDT 
*** Bug 247042 has been marked as a duplicate of this bug. ***