feat(sql): add sql formatter #166
Conversation
| return value == null || value.isEmpty(); | ||
| } | ||
|
|
||
| public static boolean equalObjects(Object o1, Object o2) { |
There was a problem hiding this comment.
Objects.equals would be fine but I think that was introduced in Java 1.7.
There was a problem hiding this comment.
As said in the description. the code is really messy, I will clean it. I first need some help on the few points I mentioned above.
|
Great, thanks!
There's a checklist here: https://github.com/diffplug/spotless/blob/master/CONTRIBUTING.md#how-to-add-a-new-formatterstep
It's not exotic, but fairly large: https://github.com/mytake/mytake/blob/master/server/src/main/resources/db/migration/V1__initial.sql
The key is for the configuration to be serializable, so that up-to-date checking works. I'm fine if it just has the config that you need - if other people need more, they can add it in future PR's.
Definitely jkiss. The DiffPlug copyright will be at the top, but the javadoc for the code should be clear that the real copyright owner is jkiss. |
baptistemesta
force-pushed
the
feat/sql_dbeaver_formatter
branch
from
7140a56
to
6850bb9
Compare
|
I updated the PR according to your feedbacks. I might still missing some copyright. Let me know if it needs some change. |
| /** Wraps up [BasicFormatterImpl](https://docs.jboss.org/hibernate/orm/4.1/javadocs/org/hibernate/engine/jdbc/internal/BasicFormatterImpl.html) as a FormatterStep. */ | ||
| public class SQLFormatterStep { | ||
|
|
||
| static final String NAME = "defaultSql"; |
There was a problem hiding this comment.
How about something that identifies its source, like dbeaverSql?
There was a problem hiding this comment.
I was not sure yep. I'll rename everything to Include DBeaver in names
| } | ||
|
|
||
| public SQLFormatterConfig sqlFormatter() { | ||
| return new SQLFormatterConfig(); |
There was a problem hiding this comment.
The name here should probably match the step name. So if we name the step dbeaverSql, then we should probably name this method dbeaver. We might add other sql formatters later, so its best to name them fairly specifically. (we've got a couple different java formatters right now, and we might have a few different Kotlin formatters soon too).
There was a problem hiding this comment.
Oh, also run spotlessApply ;-) (see CI)
This first version of the sql formatter is taken from dbeaver project.
It is configurable using closure
```
sql {
dbeaver().configFile 'sqlFormatter.properties
}
```
configuration file support following properties (with default values)
```
sql.formatter.keyword.case=UPPER
sql.formatter.statement.delimiter=;
sql.formatter.indent.type=space
sql.formatter.indent.size=4
```
baptistemesta
force-pushed
the
feat/sql_dbeaver_formatter
branch
from
6850bb9
to
4fa35e8
Compare
|
Sorry @baptistemesta, this is sat stagnant for too long. I'll make sure it's merged and released by end-of-day. |
|
Would you like any changes in how your work is credited or documented? If it looks good to you, I'll merge and release. |
|
LGTM like that :) 👍 |
|
Published in 3.7.0. |
Hi as discussed in #156 I started to implement the sql formatter using code from DBeaver.
I started from your PR #163 and basically copied code from
https://github.com/serge-rider/dbeaver/blob/af7c4eb421a5e98c7da38f5f2b70f90933a5812d/plugins/org.jkiss.dbeaver.model/src/org/jkiss/dbeaver/model/sql/format/tokenized/SQLTokenizedFormatter.java
The current state is pretty messy. I started to cleanup what is not useful from the original implementation.
There is one test that work ( com.diffplug.spotless.sql.SQLFormatterStepTest#behavior)
I have a few question to continue this work:
Thanks in advance.