You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2011/10/22 23:54:43 UTC
Removing whitespace from *.xsd and *.dtd files
Hi!
http://svn.apache.org/viewvc?rev=1187778&view=rev
affected a number of *.xsd and *.dtd files.
I think we should not touch those without a good reason.
tomcat/trunk/java/javax/servlet/jsp/resources/jsp_2_1.xsd
tomcat/trunk/java/javax/servlet/jsp/resources/jsp_2_2.xsd
tomcat/trunk/java/javax/servlet/jsp/resources/jspxml.dtd
tomcat/trunk/java/javax/servlet/jsp/resources/jspxml.xsd
tomcat/trunk/java/javax/servlet/jsp/resources/web-jsptaglibrary_2_1.xsd
tomcat/trunk/java/javax/servlet/resources/XMLSchema.dtd
tomcat/trunk/java/javax/servlet/resources/datatypes.dtd
tomcat/trunk/java/javax/servlet/resources/javaee_6.xsd
tomcat/trunk/java/javax/servlet/resources/javaee_web_services_1_3.xsd
tomcat/trunk/java/javax/servlet/resources/javaee_web_services_client_1_3.xsd
tomcat/trunk/java/javax/servlet/resources/web-app_2_2.dtd
tomcat/trunk/java/javax/servlet/resources/web-app_2_5.xsd
tomcat/trunk/java/javax/servlet/resources/web-app_3_0.xsd
tomcat/trunk/java/javax/servlet/resources/web-common_3_0.xsd
tomcat/trunk/java/javax/servlet/resources/web-fragment_3_0.xsd
tomcat/trunk/java/javax/servlet/resources/xml.xsd
Other interesting note is that this commit removed some trailing
whitespace from values in *.properties files.
Generally, this change might be visible to users, as trailing
whitespace is preserved when reading properties. Though those that I
noticed thus far look more like an editor's error - backported to tc7
in r1187816. More is likely to follow.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Mark Thomas <ma...@apache.org>.
On 23/10/2011 23:13, Konstantin Kolinko wrote:
> 2011/10/24 Mark Thomas <ma...@apache.org>:
>> On 23/10/2011 10:39, Konstantin Kolinko wrote:
>>
>>> I am -1 on applying trailing whitespaces check on *.java files.
>>>
>>> It has no practical value. It does not improve readability. I do not
>>> see what it can be useful for. It is just useless nagging.
>>
>> Trailing white-space is pointless and it bugs me. Maybe that is just me,
>> but I'd like to get rid of it.
>>
>> My original plan was to just configure my IDE to remove trailing
>> white-space when I saved a file but that led to the occasional noisy
>> commit where the white-space changes hid the real change if I forgot to
>> do a white-space only commit first. That was starting to get tricky
>> keeping track of which files I had 'fixed' and which ones I hadn't.
>>
>> Given the above, removing all of the trailing white-space in one go
>> (well, several goes as a single commit was just way too big) seemed like
>> the sensible way forward.
>>
>> In terms of benefit, it shaved ~1% of the compressed source. Nothing to
>> write home about I agree but 1% pointless fat removed it still 1%
>> pointless fat removed.
>>
>> Having gone to the trouble to remove all the trailing white-space, I'd
>> like to keep it that way and enabling the check-style check is the
>> simplest way to do that. It does mean folks working on trunk need to
>> configure their IDEs to remove trailing white-space on save for that
>> project but that doesn't seem like such a big deal.
>>
>>> There are file types where check for trailing whitespace is useful,
>>> e.g. *.properties files, (because whitespaces are not trimmed from the
>>> values that are read from the file and might be visible).
>>
>> +1
>>
>>> But for *.java files I do not see any benefits.
>>
>> In absolute terms, the benefit is minimal (~1% smaller source files) but
>> the bigger benefit for me is that it doesn't bug me any more.
>
> 1. I hate text editors that change lines that I did not touch.
Fair enough. We all have the things that drive us slowly nuts.
> 2. Do you have separate options for trunk and for 3 other branches?
Yes. I have no intention of back-porting the white-space changes to
7.0.x or earlier.
> I do not mind if your commit changes whitespace. It does not impede
> review. It is your itch, your editor, whatever. There is no technical
> merit to discuss when such commit happens.
>
> I do not like this to be enforced on people.
We can limit the checkstyle check if you feel strongly about this.
> BTW, reviewing properties changes, note the *.properties files in
> http://svn.apache.org/viewvc?limit_changes=0&view=revision&revision=1187803
>
> See "jsp.error.usebean.missing.type",
> "jsp.error.usebean.prohibited.as.session",
> "jsp.error.usebean.not.both", "jsp.error.page.nomapping.language".
>
> They all ended with ":" + space. From quick search though it looks
> that most of them, if not all, are unused.
More stuff to delete then :). I'll take a look unless you beat me to it.
>> On a related topic, unused code is next up on my list of things to clean
>> up. My plan is 1 commit to trunk to mark it deprecated. Back-port that
>> commit to 7.0.x and then remove it from trunk. I'll probably do this
>> package by package but I'll see how it goes.
>>
>
> All on the next week, or you'll wait after next 6.0.x and 7.0.x?
I wish :). I suspect that this will be a long process. I intend to do it
gradually as I have the time. My gut feeling is it will happen over months.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Mark Thomas <ma...@apache.org>.
On 23/10/2011 23:13, Konstantin Kolinko wrote:
> BTW, reviewing properties changes, note the *.properties files in
> http://svn.apache.org/viewvc?limit_changes=0&view=revision&revision=1187803
>
> See "jsp.error.usebean.missing.type",
> "jsp.error.usebean.prohibited.as.session",
> "jsp.error.usebean.not.both", "jsp.error.page.nomapping.language".
>
> They all ended with ":" + space. From quick search though it looks
> that most of them, if not all, are unused.
I went looking for an Eclipse feature / plug-in that fixes this
automatically. The closest I got was JInto but that requires some setup
and operates package by package so is a little clunky for our use case.
In the end, I installed the community edition of IntelliJ that has
unused property detection built-in. I cleaned out a bunch of old
properties and spotted a few typos / errors along the way. I'll be
committing those changes shortly.
Mark
P.S. Your were correct. None of those properties were used.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/10/24 Mark Thomas <ma...@apache.org>:
> On 23/10/2011 10:39, Konstantin Kolinko wrote:
>
>> I am -1 on applying trailing whitespaces check on *.java files.
>>
>> It has no practical value. It does not improve readability. I do not
>> see what it can be useful for. It is just useless nagging.
>
> Trailing white-space is pointless and it bugs me. Maybe that is just me,
> but I'd like to get rid of it.
>
> My original plan was to just configure my IDE to remove trailing
> white-space when I saved a file but that led to the occasional noisy
> commit where the white-space changes hid the real change if I forgot to
> do a white-space only commit first. That was starting to get tricky
> keeping track of which files I had 'fixed' and which ones I hadn't.
>
> Given the above, removing all of the trailing white-space in one go
> (well, several goes as a single commit was just way too big) seemed like
> the sensible way forward.
>
> In terms of benefit, it shaved ~1% of the compressed source. Nothing to
> write home about I agree but 1% pointless fat removed it still 1%
> pointless fat removed.
>
> Having gone to the trouble to remove all the trailing white-space, I'd
> like to keep it that way and enabling the check-style check is the
> simplest way to do that. It does mean folks working on trunk need to
> configure their IDEs to remove trailing white-space on save for that
> project but that doesn't seem like such a big deal.
>
>> There are file types where check for trailing whitespace is useful,
>> e.g. *.properties files, (because whitespaces are not trimmed from the
>> values that are read from the file and might be visible).
>
> +1
>
>> But for *.java files I do not see any benefits.
>
> In absolute terms, the benefit is minimal (~1% smaller source files) but
> the bigger benefit for me is that it doesn't bug me any more.
1. I hate text editors that change lines that I did not touch.
2. Do you have separate options for trunk and for 3 other branches?
I do not mind if your commit changes whitespace. It does not impede
review. It is your itch, your editor, whatever. There is no technical
merit to discuss when such commit happens.
I do not like this to be enforced on people.
BTW, reviewing properties changes, note the *.properties files in
http://svn.apache.org/viewvc?limit_changes=0&view=revision&revision=1187803
See "jsp.error.usebean.missing.type",
"jsp.error.usebean.prohibited.as.session",
"jsp.error.usebean.not.both", "jsp.error.page.nomapping.language".
They all ended with ":" + space. From quick search though it looks
that most of them, if not all, are unused.
>
> On a related topic, unused code is next up on my list of things to clean
> up. My plan is 1 commit to trunk to mark it deprecated. Back-port that
> commit to 7.0.x and then remove it from trunk. I'll probably do this
> package by package but I'll see how it goes.
>
All on the next week, or you'll wait after next 6.0.x and 7.0.x?
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Mark Thomas <ma...@apache.org>.
On 23/10/2011 10:39, Konstantin Kolinko wrote:
> I am -1 on applying trailing whitespaces check on *.java files.
>
> It has no practical value. It does not improve readability. I do not
> see what it can be useful for. It is just useless nagging.
Trailing white-space is pointless and it bugs me. Maybe that is just me,
but I'd like to get rid of it.
My original plan was to just configure my IDE to remove trailing
white-space when I saved a file but that led to the occasional noisy
commit where the white-space changes hid the real change if I forgot to
do a white-space only commit first. That was starting to get tricky
keeping track of which files I had 'fixed' and which ones I hadn't.
Given the above, removing all of the trailing white-space in one go
(well, several goes as a single commit was just way too big) seemed like
the sensible way forward.
In terms of benefit, it shaved ~1% of the compressed source. Nothing to
write home about I agree but 1% pointless fat removed it still 1%
pointless fat removed.
Having gone to the trouble to remove all the trailing white-space, I'd
like to keep it that way and enabling the check-style check is the
simplest way to do that. It does mean folks working on trunk need to
configure their IDEs to remove trailing white-space on save for that
project but that doesn't seem like such a big deal.
> There are file types where check for trailing whitespace is useful,
> e.g. *.properties files, (because whitespaces are not trimmed from the
> values that are read from the file and might be visible).
+1
> But for *.java files I do not see any benefits.
In absolute terms, the benefit is minimal (~1% smaller source files) but
the bigger benefit for me is that it doesn't bug me any more.
On a related topic, unused code is next up on my list of things to clean
up. My plan is 1 commit to trunk to mark it deprecated. Back-port that
commit to 7.0.x and then remove it from trunk. I'll probably do this
package by package but I'll see how it goes.
Cheers,
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/10/23 Mark Thomas <ma...@apache.org>:
> On 22/10/2011 23:33, Konstantin Kolinko wrote:
>> 2011/10/23 Mark Thomas <ma...@apache.org>:
>>> On 22/10/2011 22:54, Konstantin Kolinko wrote:
>>>> Hi!
>>>>
>>>> http://svn.apache.org/viewvc?rev=1187778&view=rev
>>>> affected a number of *.xsd and *.dtd files.
>>>>
>>>> I think we should not touch those without a good reason.
>
> <snip/>
>
>>> I don't see the harm in removing the trailing white-space from these
>>> files. It shouldn't change their meaning.
>>
>> From technical point of view it should not. But all of them originate
>> from somewhere, and I would not want them to differ from their
>> originals more than necessary.
>
> That is a fair point.
>
> My own preference is for not having to list them as exclusions in the
> checkstyle config. Thinking about it, I'm not sure how easy that would
> be. It is easy enough to filter by file extension but that doesn't quite
> work in this case. I'd rather not have a separate checkstyle file just
> for trailing white-space but I don't see another way to exclude the spec
> files.
>
> Given that diff tools can be easily configured to ignore white-space I'm
> leaning towards leaving things as they currently are.
I am -1 on applying trailing whitespaces check on *.java files.
It has no practical value. It does not improve readability. I do not
see what it can be useful for. It is just useless nagging.
There are file types where check for trailing whitespace is useful,
e.g. *.properties files, (because whitespaces are not trimmed from the
values that are read from the file and might be visible). But for
*.java files I do not see any benefits.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Mark Thomas <ma...@apache.org>.
On 22/10/2011 23:33, Konstantin Kolinko wrote:
> 2011/10/23 Mark Thomas <ma...@apache.org>:
>> On 22/10/2011 22:54, Konstantin Kolinko wrote:
>>> Hi!
>>>
>>> http://svn.apache.org/viewvc?rev=1187778&view=rev
>>> affected a number of *.xsd and *.dtd files.
>>>
>>> I think we should not touch those without a good reason.
<snip/>
>> I don't see the harm in removing the trailing white-space from these
>> files. It shouldn't change their meaning.
>
> From technical point of view it should not. But all of them originate
> from somewhere, and I would not want them to differ from their
> originals more than necessary.
That is a fair point.
My own preference is for not having to list them as exclusions in the
checkstyle config. Thinking about it, I'm not sure how easy that would
be. It is easy enough to filter by file extension but that doesn't quite
work in this case. I'd rather not have a separate checkstyle file just
for trailing white-space but I don't see another way to exclude the spec
files.
Given that diff tools can be easily configured to ignore white-space I'm
leaning towards leaving things as they currently are.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/10/23 Mark Thomas <ma...@apache.org>:
> On 22/10/2011 22:54, Konstantin Kolinko wrote:
>> Hi!
>>
>> http://svn.apache.org/viewvc?rev=1187778&view=rev
>> affected a number of *.xsd and *.dtd files.
>>
>> I think we should not touch those without a good reason.
>>
>> tomcat/trunk/java/javax/servlet/jsp/resources/jsp_2_1.xsd
>> tomcat/trunk/java/javax/servlet/jsp/resources/jsp_2_2.xsd
>> tomcat/trunk/java/javax/servlet/jsp/resources/jspxml.dtd
>> tomcat/trunk/java/javax/servlet/jsp/resources/jspxml.xsd
>> tomcat/trunk/java/javax/servlet/jsp/resources/web-jsptaglibrary_2_1.xsd
>>
>> tomcat/trunk/java/javax/servlet/resources/XMLSchema.dtd
>> tomcat/trunk/java/javax/servlet/resources/datatypes.dtd
>> tomcat/trunk/java/javax/servlet/resources/javaee_6.xsd
>> tomcat/trunk/java/javax/servlet/resources/javaee_web_services_1_3.xsd
>> tomcat/trunk/java/javax/servlet/resources/javaee_web_services_client_1_3.xsd
>> tomcat/trunk/java/javax/servlet/resources/web-app_2_2.dtd
>> tomcat/trunk/java/javax/servlet/resources/web-app_2_5.xsd
>> tomcat/trunk/java/javax/servlet/resources/web-app_3_0.xsd
>> tomcat/trunk/java/javax/servlet/resources/web-common_3_0.xsd
>> tomcat/trunk/java/javax/servlet/resources/web-fragment_3_0.xsd
>> tomcat/trunk/java/javax/servlet/resources/xml.xsd
>
> I don't see the harm in removing the trailing white-space from these
> files. It shouldn't change their meaning.
>From technical point of view it should not. But all of them originate
from somewhere, and I would not want them to differ from their
originals more than necessary.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Removing whitespace from *.xsd and *.dtd files
Posted by Mark Thomas <ma...@apache.org>.
On 22/10/2011 22:54, Konstantin Kolinko wrote:
> Hi!
>
> http://svn.apache.org/viewvc?rev=1187778&view=rev
> affected a number of *.xsd and *.dtd files.
>
> I think we should not touch those without a good reason.
>
> tomcat/trunk/java/javax/servlet/jsp/resources/jsp_2_1.xsd
> tomcat/trunk/java/javax/servlet/jsp/resources/jsp_2_2.xsd
> tomcat/trunk/java/javax/servlet/jsp/resources/jspxml.dtd
> tomcat/trunk/java/javax/servlet/jsp/resources/jspxml.xsd
> tomcat/trunk/java/javax/servlet/jsp/resources/web-jsptaglibrary_2_1.xsd
>
> tomcat/trunk/java/javax/servlet/resources/XMLSchema.dtd
> tomcat/trunk/java/javax/servlet/resources/datatypes.dtd
> tomcat/trunk/java/javax/servlet/resources/javaee_6.xsd
> tomcat/trunk/java/javax/servlet/resources/javaee_web_services_1_3.xsd
> tomcat/trunk/java/javax/servlet/resources/javaee_web_services_client_1_3.xsd
> tomcat/trunk/java/javax/servlet/resources/web-app_2_2.dtd
> tomcat/trunk/java/javax/servlet/resources/web-app_2_5.xsd
> tomcat/trunk/java/javax/servlet/resources/web-app_3_0.xsd
> tomcat/trunk/java/javax/servlet/resources/web-common_3_0.xsd
> tomcat/trunk/java/javax/servlet/resources/web-fragment_3_0.xsd
> tomcat/trunk/java/javax/servlet/resources/xml.xsd
I don't see the harm in removing the trailing white-space from these
files. It shouldn't change their meaning.
> Other interesting note is that this commit removed some trailing
> whitespace from values in *.properties files.
>
> Generally, this change might be visible to users, as trailing
> whitespace is preserved when reading properties. Though those that I
> noticed thus far look more like an editor's error - backported to tc7
> in r1187816. More is likely to follow.
Given that trailing white-space is extremely hard to see in .properties
files, I doubt that any of it was deliberate. I certainly can't recall
an instance where trailing white-space necessary.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org