I’ve investigated further.  I think that at some point I must have modified the 
imported felix eclipse template.

However, 
- the template is out of date, there are a lot of new language features with 
formatting settings.
- the template doesn’t match the scr code very well.

I’ve pushed a couple branches to GitHub:

commit 
https://github.com/djencks/felix/commit/bbc1bcff31c0d375701dc8ef7a8c0d2c51a958b3
 is the result of reformatting scr with a freshly imported v11 template.  There 
seem to me to be an unreasonable number of changes.

commit 
https://github.com/djencks/felix/commit/2724700da4699c1428e8779e7e35a08a4622f368
 is the result of reformatting scr with a proposed updated template.  Exporting 
the v11 template and the proposed v12 template, sorting, and diffing I get

MacBook-Pro-2:felix david$ diff template-11-exported-s.xml 
template-12-a-exported-s.xml 
158,159c158,159
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_cast" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_catch" 
value="do not insert"/>
---
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_cast" 
> value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_catch" 
> value="insert"/>
162,163c162,163
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_for" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_if" 
value="do not insert"/>
---
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_for" 
> value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_if" 
> value="insert"/>
165,170c165,170
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_method_invocation"
 value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_parenthesized_expression"
 value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_switch" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_synchronized"
 value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_try" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_while" 
value="do not insert"/>
---
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_method_invocation"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_parenthesized_expression"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_switch"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_synchronized"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_try" 
> value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_while" 
> value="insert"/>
189,190c189,190
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_cast" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_catch" 
value="do not insert"/>
---
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_cast" 
> value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_catch"
>  value="insert"/>
193,194c193,194
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_for" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_if" 
value="do not insert"/>
---
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_for" 
> value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_if" 
> value="insert"/>
196,201c196,201
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_method_invocation"
 value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_parenthesized_expression"
 value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_switch" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_synchronized"
 value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_try" 
value="do not insert"/>
< <setting 
id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_while" 
value="do not insert"/>
---
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_method_invocation"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_parenthesized_expression"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_switch"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_synchronized"
>  value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_try" 
> value="insert"/>
> <setting 
> id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_while"
>  value="insert"/>
283c283
< <setting id="org.eclipse.jdt.core.formatter.lineSplit" value="90"/>
---
> <setting id="org.eclipse.jdt.core.formatter.lineSplit" value="120"/>


I think it’s a bit odd that constructor and method declarations don’t have 
spaces (Type var) whereas invocations do ( new Foo( x ) )  There is a lot of 
inconsistency in the code but I think there are more uses without spaces than 
with at the moment.  I’d prefer they all have spaces.

I’d suggest that whether or not we adopt my proposed changes we should:

- put the template in source control so we can track changes to it
- sort it so diffs make sense.

If anyone thinks proceeding is a good idea I’ll open an issue and attach the 
two sorted updated templates.

thanks
david jencks



> On Apr 25, 2017, at 2:51 PM, Thomas Watson <[email protected]> wrote:
> 
> I much prefer we use the eclipse formatting template.  I have that template
> setup for my workspace that I use for felix.  I have also noticed the code
> gets formatted on save.  Looking at my workspace settings it does seem this
> is enabled to format the code on save.  I am unsure if I set that myself or
> of the act of importing the formatting template did it.
> 
> Tom.
> 
> On Tue, Apr 25, 2017 at 11:09 AM, David Jencks <[email protected]>
> wrote:
> 
>> Hi Christian,
>> 
>> Felix has such an eclipse fomatting template, apparently my problem is
>> that I tried to use it.  Perhaps it should be put in a more accessible
>> place?  I think we should either use it or remove it, I’m a bit in favor of
>> using it.
>> 
>> at the end of http://felix.apache.org/documentation/development/
>> coding-standards.html there is a link to a template at
>> 
>> https://issues.apache.org/jira/secure/attachment/
>> 12419890/Apache+Felix+Eclipse+Template.xml which is the one I’ve been
>> using.
>> 
>> There’s also https://issues.apache.org/jira/browse/FELIX-1406 which has
>> two attachments….
>> 
>> https://issues.apache.org/jira/secure/attachment/12419862/
>> ApacheFelixEclipseTemplate.xml
>> https://issues.apache.org/jira/secure/attachment/12419890/Apache%20Felix%
>> 20Eclipse%20Template.xml (this is the one linked from the
>> coding-standards page)
>> 
>> Reading the comments and checking links, I think that this issue has
>> resulted in the current target of the coding-standards page link being the
>> 2nd attachment to the issue.
>> 
>> thanks
>> david jencks
>> 
>> 
>>> On Apr 24, 2017, at 11:43 PM, Christian Schneider <
>> [email protected]> wrote:
>>> 
>>> Hi David,
>>> 
>>> this might be an eclipse config.
>>> Open preferences and go to Java / Editor / Save Actions.
>>> There you can select to format the code on save of a file.
>>> Maybe this option is switched on for you.
>>> 
>>> That said I think it would make sense to have a common formatter for the
>> code. In CXF we have checkstyle checks for the formatting as well
>>> as an eclipse formatter. It helps a lot to make sure that the code is
>> formatted in the same way by everyone and makes it easier to spot actual
>> changes.
>>> On the other hand the cxf formatting rules are a bit strange regarding
>> imports so without all the formatters installed it is a pain... So like
>>> most of the time it is a two edged sword ...
>>> 
>>> Christian
>>> 
>>> 
>>> 
>>> On 25.04.2017 03:51, David Jencks wrote:
>>>> For reasons unclear to me Eclipse has started saving modified files
>> reformatted with the felix eclipse code template.  This, to put it mildly,
>> has shown that the code isn’t formatted with  the template, and I can’t
>> fish out my actual changes by hand.
>>>> 
>>>> What would people think if I reformatted the entire DS project and
>> checked in the result?  I’m hoping that I can convince git to end up
>> showing me my actual changes after this exercise.
>>>> 
>>>> In a somewhat separate issue, I have so far been unable to convince
>> Eclipse to let me open the R7 branch, so it’s possible someone else may
>> have to apply similar changes to that.
>>>> 
>>>> thanks
>>>> david jencks
>>>> 
>>> 
>>> 
>>> --
>>> Christian Schneider
>>> http://www.liquid-reality.de
>>> 
>>> Open Source Architect
>>> http://www.talend.com
>>> 
>> 
>> 

Reply via email to