enhanced keyword - corrections

[wumpz]

Since the discussion in #1382 seems to be stopped here is a list of things, that still need to be addressed:

• remove JavaCC dependency from JSqlParser.jar, switch back to
• remove the need to run this generation keyword task. We agreed that simply adding to the grammar should be enough, but at the moment it isn't.
  • my counter-proposal needs some additional work as well: either add your new keyword to the restricted list or add it to the relobj production, however this is enough to succeed in tests without rerunning some kind of generation task without some precompiling and stuff. However, for someone working with a JavaCC grammar file this work is straightforward. So you see I am going in direction of compromise, but for the first PRs comming in this PR is making implementing things worse, IMHO. Even you had to step in and correct PRs.
• try to preserve comments and stuff from relobj productions
• reference to this sphinx documentation: where on github should this website run and how to edit it. I don't like always referencing your server.

So if those things cannot be addressed I will disable those tests failing tests or maybe you could run it with your generation task. Or maybe you have a better approach to this.

[wumpz]

To be honest, for me this is a blocker before releasing the next version.

[manticore-projects]

Thank you for the summary.

* remove JavaCC dependency from JSqlParser.jar, switch back to

Back to what? Regular Expressions?
Isn't there are solution when we have a JavaCC dependency, but just exclude it from packaging?

* remove the need to run this generation keyword task. We agreed that simply adding to the grammar should be enough, but at the moment it isn't.

Running this task is already completely optional.

 for the first PRs comming in this PR is making implementing things worse, IMHO. Even you had to step in and correct PRs.

I do not agree. The problem was only about TIMING.
Those PR's were issued before the Keywords PR's have been accepted. Only that way they passed the Integration Tests and failed to build then later.

It won't occur again since everything is in place now.
The relevant problem is: thing get accepted too slow.

* try to preserve comments and stuff from relobj productions

I am supportive to that. Please elaborate, what is missing please.

* reference to this sphinx documentation: where on github should this website run and how to edit it. I don't like always referencing your server.

Your project, your decision. I have only bridged the gap since no other server/host has been available.

So if those things cannot be addressed I will disable those tests failing tests or maybe you could run it with your generation task. Or maybe you have a better approach to this.

I am not even clear where exactly the problem is. Currently the process works exactly as intented and discussed:

1. Contributor amends the Grammar and adds a new Token (not knowing about Keywords)
2. Keyword tests will fail and request for running the Keyword task
3. Contributor runs the updateKeyword task
4. all tests succeed
5. PR can be committed
6. Project owner runs site and upload tasks to build new Website and upload to the server

Please re-define how this process should work instead.

[wumpz]

Sorry about the late answer. I got some medical issues here :/.

Back to what? Regular Expressions?
Isn't there are solution when we have a JavaCC dependency, but just exclude it from packaging?

Move it to the tests or we could build a separate module. There the dependency is not a problem. But if a first step is to got to the regexp, then so be it. Excluding a dependency or marking it as provided is not a clean solution. In every case a link to JavaCC would be remain within the meta information of the jar.

Running this task is already completely optional.

Its not. Simply adding to the grammar is not enough. As you described, someone has to run the updateKeyword task or has to adapt your keyword list. (look at my long comment above.)
This generates so much friction to make changes to the grammar for new developers. (I don't think my proposal is here much of an improvement. ) Again, we agreed that simply adding to the grammar should be enough. So someone should be able to change the RelObj productions, introduce new tokens without breaking the "normal" roundtrip. So I think a better way would be to keep this keyword build step in deed option and in our hands and run it afterward to introduce multiple new whitelisted keywords in one step. This could be ensured by reconfigured build pipeline (some profile or so). Then this would be some kind of tool to use and would not interfere the building process.

Your project, your decision. I have only bridged the gap since no other server/host has been available.

Thanks for that. So how could we move this to github.io? I have never done this using Sphinx, but maybe this helps: https://www.docslikecode.com/articles/github-pages-python-sphinx/.

[manticore-projects]

Sorry about the late answer. I got some medical issues here :/.

Das tut mir leid, ich wuensche alles Gute! Toi toi toi!

On the JavaCC dependency:
Move it to the tests or we could build a separate module. There the dependency is not a problem.

I think, separate module is the way to go. I will prepare a solution for this.

Thanks for that. So how could we move this to github.io? I have never done this using Sphinx, but maybe this helps: https://www.docslikecode.com/articles/github-pages-python-sphinx/.

I will prepare a Maven Task for Sphinx/Upload to github.io

[manticore-projects]

Thanks for that. So how could we move this to github.io?

I have added a Github Action to PR #1624 , that builds the Website on Commit to Master and deploys the static Site to GH Pages.

Demo: https://manticore-projects.github.io/JSqlParser/

[manticore-projects]

• remove JavaCC dependency from JSqlParser.jar, switch back to

Solved via Commit 15c543e in PR #1624.
The Tokens of the Grammar are found via Regular Expression. JavaCC is now only a Test Dependency in order to proof completeness and correctness of the found Tokens during the tests.

[manticore-projects]

Status

• remove JavaCC dependency

• remove the need to run this generation keyword task.
  Nothing to do, this step is optional.
  It can be used, after new token were introduced as an alternative to editing RelObjectNameWithoutValue manually.

• try to preserve comments and stuff from relobj productions
  RelObjectNameWithoutValue does not contain any comments and never did. We can think about it, when there is an actual need to comment something in this production.

• reference to this sphinx documentation: where on github should this website run and how to edit it. I don't like always referencing your server.
  Solved via GitHub Action.

• remove CSS Checks from Codacy (still failing)

[manticore-projects]

Beside the 3 Open PRs, which should be considered since they fix material, long standing bugs I would like to incorporate 2 major changes before a 4.6 release:

1. Standardized Java Formatting step during based on Maven Spotify Plugin and Eclipse Formatter. The configuration of course must represent the Checkstyle Rules.

2. Reformat the whole code according to those rules.

Ideally this change would be encapsulated into one single PR with no other changes but re-formatting. And it should be fast tracked since it would invalidate any other pending changes.

[manticore-projects]

Please can I have write/commit rights?

[wumpz]

Java format is standardized due to checkstyle plugin. However automatic formatting is a good step.

Spotify plugin? Do you mean spotless plugin? I will not introduce some kind of eclipse formatter setup. I propose google code style.

[manticore-projects]

Spotify plugin? Do you mean spotless plugin?

yes, typo.

I will not introduce some kind of eclipse formatter setup. I propose google code style.

yes, google code style.

[wumpz]

https://jsqlparser.github.io/JSqlParser is online. Nice job.

[manticore-projects]

Glad you like it, although i will need to provide some more fine tuning -- now that most of the PRs got merged.
It has been a good year for JSQLParser, I think.

[wumpz]

I think so too. However, there are some glitched I tried to avoid but was not able to. ;) Somewhere you mentioned already
deeper problems of the grammar, which could only be solved with a heavy refactoring.

[manticore-projects]

Closing this, since 4.6 has been released.