I've just recreated EclipseCodeFormat.xml from our old Eclipse's settings (as I did already when I removed the settings from the repository).

Result: When I reformat Application.java I get the same line breaks as Igor has reported. Additionally all comments are limited to 80 characters length :/.

Apparently there's a bug in Eclipse's export or import functionality for formatting.

Sven


On 01/29/2014 07:35 AM, Igor Vaynberg wrote:
then change eclipse formatter settings to match whatever is in our
code base now?

-igor

On Tue, Jan 28, 2014 at 10:21 AM, Martin Grigorov <[email protected]> wrote:
On Tue, Jan 28, 2014 at 7:13 PM, Igor Vaynberg <[email protected]>wrote:

the format edited lines only applies to save actions, it will do
nothing to help me with my ctrl+shift+f twitch. i often format code as
i work on it because i just brain dump it all into a single line and
let the ide format it by pressing ctrl+shift+f. this happens multiple

Just let it format that line by pressing ctrl+s instead :)
it is easier - one button less to press, and does less "damages"


times before saving.

anyways, we need consistent formatting in all our code. once that is

I am a bit worried about Git cherry-pick after the major reformat


done i can go back to working on the queuing thing.

-igor

On Tue, Jan 28, 2014 at 9:04 AM, Sven Meier <[email protected]> wrote:
"format edited lines" are configured in org.eclipse.jdt.ui.prefs, and
these
files are no longer under version control.

Sven



On 01/28/2014 05:35 PM, Martin Grigorov wrote:
I think the setting "format edited lines" is not in the committed
.settings/ and that's why Igor's IDE touches code that it should not.

I am +1 to reformat all files now and have cleaner commit diffs in the
future.
But we should do this for 1.5, 6.x and 7.x. Otherwise I think Git will
have
big problems with merging/cherry picking.

Martin Grigorov
Wicket Training and Consulting


On Tue, Jan 28, 2014 at 5:27 PM, Sven Meier <[email protected]> wrote:

   if the formatter config is correct i shouldnt have to just format
edited
lines...

Correct.

It's just that we've coded with difference format settings / IDEs in
the
past years. To be sure we'd have to run the formatter once on all
files.
Sven



On 01/28/2014 05:12 PM, Igor Vaynberg wrote:

if the formatter config is correct i shouldnt have to just format
edited lines...

-igor

On Tue, Jan 28, 2014 at 4:30 AM, Sven Meier <[email protected]> wrote:

Our format defines lineSplit=100, so that lines gets wrapped
correctly.
If you're using Eclipse's Save Actions, do you have "format edited
lines"
selected in the configuration?

Regards
Sven



On 01/28/2014 09:52 AM, Igor Vaynberg wrote:

apparently eclipse formatter setup in master is incorrect. i am
working on a new queuing implementation idea and keep getting crap
like this all over the code, any ideas? after running code cleanup
on
the workspace all the files are modified and i have effectively lost
my changes...

-igor

diff --git
a/wicket-core/src/main/java/org/apache/wicket/Application.
java
b/wicket-core/src/main/java/org/apache/wicket/Application.java
index 7d8e52b..eab5a42 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Application.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Application.java
@@ -155,7 +155,8 @@ public abstract class Application implements
UnboundListener, IEventSink
             * without being in a request/ being set in the thread
local
(we need that e.g. for when we are
             * in a destruction thread).
             */
-       private static final Map<String, Application>
applicationKeyToApplication = Generics.newHashMap(1);
+       private static final Map<String, Application>
applicationKeyToApplication = Generics
+               .newHashMap(1);

            /** Log. */
            private static final Logger log =
LoggerFactory.getLogger(Application.class);
@@ -219,8 +220,8 @@ public abstract class Application implements
UnboundListener, IEventSink
                    Application application =
ThreadContext.getApplication();
                    if (application == null)
                    {
-                       throw new WicketRuntimeException("There is
no
application attached to current thread " +
-                               Thread.currentThread().getName());
+                       throw new WicketRuntimeException("There is
no
application attached to current thread "
+                               + Thread.currentThread().getName());

On Thu, Nov 14, 2013 at 3:07 AM, Martijn Dashorst
<[email protected]> wrote:

I did not realise this was waiting on me.

I guess the main problem with using the resources bundle approach
is
that the formatting.xml remains necessary for compatibility with
IntelliJ (and perhaps Netbeans). So we can't just bundle up the
.settings folder and use that as the canonical version.

Martijn


On Tue, Nov 12, 2013 at 11:10 PM, Igor Vaynberg <
[email protected]>
wrote:

any progress Martijn?

-igor

On Sun, Nov 10, 2013 at 12:49 AM, Martijn Dashorst
<[email protected]> wrote:

We can let the eclipse plugin automatically add the project
settings
if we upload a jar to maven central with our configuration.

<plugin>
<artifactId>maven-eclipse-plugin</artifactId>
<version>2.9</version>
<inherited>true</inherited>
<configuration>
<downloadSources>true</downloadSources>
<downloadJavadoc>false</downloadJavadoc>
<ajdtVersion>${java.version}</ajdtVersion>
<additionalConfig>
<file>
<name>.settings/edu.umd.cs.findbugs.plugin.eclipse.prefs</name>
<location>/edu.umd.cs.findbugs.plugin.eclipse.prefs</location>
</file>
<file>
<name>.settings/org.eclipse.core.resources.prefs</name>
<location>/org.eclipse.core.resources.prefs</location>
</file>
<file>
<name>.settings/org.eclipse.jdt.apt.core.prefs</name>
<location>/org.eclipse.jdt.apt.core.prefs</location>
</file>
<file>
<name>.settings/org.eclipse.jdt.core.prefs</name>
<location>/org.eclipse.jdt.core.prefs</location>
</file>
<file>
<name>.settings/org.eclipse.jdt.ui.prefs</name>
<location>/org.eclipse.jdt.ui.prefs</location>
</file>
<file>
<name>.settings/org.eclipse.wst.validation.prefs</name>
<location>/org.eclipse.wst.validation.prefs</location>
</file>
<file>
<name>.settings/org.maven.ide.eclipse.prefs</name>
<location>/org.maven.ide.eclipse.prefs</location>
</file>
</additionalConfig>
</configuration>
<dependencies>
<dependency>
<groupId>nl.topicus.onderwijs</groupId>
<artifactId>eclipse-settings</artifactId>
<version>2012.2.2</version>
</dependency>
</dependencies>
</plugin>

On Sun, Nov 10, 2013 at 12:45 AM, Igor Vaynberg
<[email protected]> wrote:

yes, making it a workspace default messes up other projects...

this way every time i import a project into the eclipse
workspace
i
have to go and manually set the formatter on every module, which
as
you can imagine is not optimal....

-igor

On Sat, Nov 9, 2013 at 1:40 PM, Martin Grigorov <
[email protected]>
wrote:

But you have to import the xml just once, right ? It is not a
big
deal.
Or the problem is that the xml messes up the other projects in
your
workspace ?



On Sat, Nov 9, 2013 at 7:24 AM, Igor Vaynberg
<[email protected]>wrote:

   it is really frustrating that i have to do this manually now.
before
all i had to do was checkout the project and it was all set.
wicket
shares my workspace with other projects so the
workspace-default
is
not going to work.

can we drop the format def on wicket.apache.org and configure
the
maven plugin to set it up:


http://maven.apache.org/plugins/maven-eclipse-plugin/
examples/load-code-styles.html

-igor

On Fri, Nov 8, 2013 at 12:56 AM, Martin Grigorov
<[email protected]>
wrote:

I'll test this soon.
I'll update the docs for IDEA too if needed.


On Thu, Nov 7, 2013 at 11:02 AM, Sven Meier <[email protected]
wrote:

   Thanks, I've added a hint to the Idea instructions.
Regards
Sven


On 11/06/2013 10:12 AM, Vojtěch Krása wrote:

   You should also specify values for "Class count to use
import
with '*'"
and
"Names count to use static import with '*'", since these
values
are
not in EclipseCodeFormat.xml,
and differs between Idea and Eclipse by default.


V.


2013/11/6 Sven Meier <[email protected]>

     Hi all,

I removed all org.eclipse.jdt.[core|ui].prefs from the
repo
as
discussed.
EclipseCodeFormat.xml is updated now to our latest and
greatest
code
format
(which might differ between 6.x and master).

Eclipse users should run "mvn eclipse:eclipse" to
regenerate
org.eclipse.jdt.core.prefs, then (re-)import
EclipseCodeFormat.xml and
use
it as the default for your Wicket workspace(s).
I've added a paragraph about the recommended Eclipse setup
here:
http://wicket.apache.org/learn/ides.html

Could an Idea user please confirm that the format plugin (
http://plugins.jetbrains.com/plugin/6546) works as
expected?
Regards
Sven

On 11/05/2013 12:05 PM, Martin Grigorov wrote:

     On Tue, Nov 5, 2013 at 1:01 PM, Sven Meier <
[email protected]>
wrote:

      IMHO we should have one authoritative source for our
source
format
only.

   Whether this is EclipseCodeFormat.xml or something else
can
be

dicussed
   on
the other mail thread.

Currently all org.eclipse.jdt.core.prefs have already
diverged
from
EclipseCodeFormat.xml (perhaps they even differ between
each

other?),
   so
I'm +1 to remove those settings from the repo as Martin
has

suggested.
   I can live with having to configure my Wicket workspace(s)
once by
importing EclipseCodeFormat.xml.

So if no one objects, I'll update EclipseCodeFormat.xml
from
the
current
settings in wicket-core and apply Martin's patch
afterwards.

     I'm +1.

With the plugin that Rusi suggested in the other thread I
can
import
       EclipseCodeFormat.xml in Intellij IDEA and
hopefully
the

formatting
   will
be the same for all of us.



     Sven

On 11/04/2013 04:42 PM, Martin Funk wrote:

      not quite

   if the org.eclipse.jdt.ui.prefs are not present
eclipse
will
fall

back
   to
the workspace setting esp. formatter.
The formatter profile as I described it in the
attachment
to
https://issues.apache.org/jira/browse/WICKET-5399
has to be imported into the workspace once.
If one has to follow more than one code formatting
rulesets,
than

they
   have to be set for each
project. The setting of the formatter profile will be
written
to
org.eclipse.jdt.ui.prefs.

mf

Am 04.11.2013 um 16:25 schrieb Sven Meier
<[email protected]
:
       Ok, removing org.eclipse.jdt.core.prefs and
org.eclipse.jdt.ui.prefs
is

     easy.

But without these files the Eclipse project settings
(Java
Code

Style
   ->
Formatter) have to be adjusted manually for each
Wicket
module

after
   "mvn
eclipse:eclipse" :(.

Sven

On 11/04/2013 09:58 AM, Martin Grigorov wrote:

      Hi,

   Can someone of other Wicket code developers take a
look
at
https://github.com/apache/wicket/pull/56 ?
This is a pull request with some changes/updates to
Eclipse's
.settings/
(required by newer versions of Eclipse ?!).
I don't use Eclipse and I cannot decide whether the
PR
is
good or
not.

https://github.com/apache/wicket/pull/57/commits is
another
PR

from
   Martin
Funk that has some improvements to Wicket's unit
tests
that
I'd

like
   to
merge but I cannot because it depends on PR 56.

Additionally I'd like to ask all Eclipse users to
disable
the

"auto
   format
the whole file" feature.
https://github.com/mafulafunk/wicket/commit/
0aac81f393047865088864c6b299ce1e022ce1fa
(part
of PR 57) has such formatting changes that we agreed
should
not

be
   together
with functional changes because they add a lot of
noise
that

makes
   the
code
review and git bisect sesssions a lot harder.
Lately I have seen such changes in Sven's commits as
well.

Please configure Eclipse to not auto format or to
format
only the
changed
code, but not the whole file.
If this is not possible with Eclipse then you can use
"git
add

-p"
   to
select only the functional changes in one commit and
all

formatting
   related
ones in another one.

Thanks!

On Sun, Nov 3, 2013 at 11:40 PM, mafulafunk
<[email protected]>
wrote:

       GitHub user mafulafunk opened a pull request:

            https://github.com/apache/wicket/pull/57

           Assert that instance of

           Ok,

           this is two commits aa422c1 is just
because
the
eclipse
property
files
get in the way.

           The commit 0aac81f was inspired by a non
informativ test
fail.
           Like the assert
           assertTrue(factory.getFieldValue(field,
obj)
instanceof
ILazyInitProxy);
           simply fails with no further information.
           As org.hamcrest.CoreMatchers is already
pulled
into the
classpath
by
junit it might be ok to transform the given
assertTrue
to:
           assertThat(factory.getFieldValue(field,
obj),
instanceOf(ILazyInitProxy.class));

           Now when the assertion fails the value of
the
first
argument is
printed
           in the test output.

You can merge this pull request into a Git
repository
by

running:
             $ git pull https://github.com/mafulafunk/
wicketassertThatInstanceOf

Alternatively you can review and apply these changes
as
the

patch
    at:

https://github.com/apache/wicket/pull/57.patch

----
commit aa422c16a8711c43e03b65cec7148afd53153ac5
Author: Martin Funk <[email protected]>
Date:   2013-10-28T19:03:09Z

           remove eclipse jdt.core and jdt.ui prefs

commit 0aac81f393047865088864c6b299ce1e022ce1fa
Author: Martin Funk <[email protected]>
Date:   2013-11-03T21:20:56Z

           Refactor Testcases to make failing tests
more

informative:
             Refactor
           assertTrue(factory.getFieldValue(field,
obj)
instanceof
ILazyInitProxy);
           to
           assertThat(factory.getFieldValue(field,
obj),
instanceOf(ILazyInitProxy.class));

           Now when the assertion fails the value of
the
first
argument is
printed
           in the test output.

----






--
Become a Wicket expert, learn from the best:
http://wicketinaction.com

--
Become a Wicket expert, learn from the best:
http://wicketinaction.com


Reply via email to