D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:3ebb3c09e512: SQL: various improvements and fix 
if/case/loop/end detection with SQL (Oracle) (authored by jpoelen).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D11470?vs=32861&id=40010#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=32861&id=40010

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml

To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, cullmann, rkflx, michaelh, kevinapavew, 
ngraham, bruns, demsking, sars, dhaumann

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  I guess ok - if issues arise, we have to fix them :-)

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  sql2

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, cullmann, rkflx, michaelh, kevinapavew, 
ngraham, bruns, demsking, sars, dhaumann

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen added a comment.


  There have been changes in the xml files that will cause conflicts. I would 
look in the next few days.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, cullmann, rkflx, michaelh, kevinapavew, 
ngraham, bruns, demsking, sars, dhaumann

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.
Restricted Application added a project: Kate.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Beside the short arc confusion, the current state can go in, or?

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, cullmann, rkflx, michaelh, kevinapavew, 
ngraham, bruns, demsking, sars, dhaumann, #frameworks

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen added a comment.


  The page on Phabricator is much better than before. My problem was about 
creating a branch without the `-t` option which requires me to specify a 
commit/branch to do the diff.
  
  I also realized later that I was reloading a page from the revision history, 
so I always saw the same bad file list and did not understand why. In fact, the 
last 3 diff are exactly the same.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: rkflx, #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[Henrik Fehlauer]

rkflx added a comment.


  Sorry for chiming in, but in light of the focus goal T7116: Streamlined 
onboarding of new contributors  it's 
important to fix rough edges in our infrastructure, too:
  
  In D11470#252163 , @jpoelen wrote:
  
  > I am completely lost with `arc diff`. The command adds files that I did not 
normally modify in this branch and specifiying a reference commit does not seem 
to work.
  
  
  Do you know about our recommended workflow in 
https://community.kde.org/Infrastructure/Phabricator#Workflow? Happy to help 
with that, just let me know.
  
  > I ended up doing everything through a new branch, sorry to rot the history 
like this :/.
  
  Phabricator's Differential is simply a list of patch iterations if you will, 
there is no "history" as such which you might have impacted. Only if you use 
the web uploader instead of Arcanist you make life for the reviewer a bit 
harder (no author information when landing, no context in the diff view, no 
base commit).

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: rkflx, #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen updated this revision to Diff 32861.
jpoelen added a comment.


  I am completely lost with `arc diff`. The command adds files that I did not 
normally modify in this branch and specifiying a reference commit does not seem 
to work.
  
  I ended up doing everything through a new branch, sorry to rot the history 
like this :/.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=32860&id=32861

BRANCH
  sql2

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen updated this revision to Diff 32860.
jpoelen added a comment.


  - SQL: various improvements and fix if/case/loop/end detection with SQL 
(Oracle)
  - add sql tests
  - update kateversion to 5.0 and allows multiple spaces for "end if", "end 
case" and "end loop"
  - autotests/input/highlight.sh.syntax and autotests/input/highlight.sh are 
restored

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=32855&id=32860

BRANCH
  sql2

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen updated this revision to Diff 32855.
jpoelen added a comment.


  - diff with the proper branch

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=32853&id=32855

BRANCH
  new_sql_oracle

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen updated this revision to Diff 32853.
jpoelen added a comment.


  - autotests/input/highlight.sh.syntax and autotests/input/highlight.sh are 
restored

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=32852&id=32853

BRANCH
  new_sql_oracle

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/highlight.cmake.fold
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml
  src/indexer/katehighlightingindexer.cpp
  src/lib/rule.cpp
  src/lib/rule_p.h

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen updated this revision to Diff 32852.
jpoelen added a comment.


  - SQL: various improvements and fix if/case/loop/end detection with SQL 
(Oracle)
  - add sql tests
  - update kateversion to 5.0 and allows multiple spaces for "end if", "end 
case" and "end loop"

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=29866&id=32852

BRANCH
  new_sql_oracle

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/highlight.cmake.fold
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/highlight.sh
  autotests/input/highlight.sh.syntax
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml
  src/indexer/katehighlightingindexer.cpp
  src/lib/rule.cpp
  src/lib/rule_p.h

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[Dominik Haumann]

dhaumann requested changes to this revision.
dhaumann added a comment.
This revision now requires changes to proceed.


  Looks mostly good, but there are some minor issues we first need to address:
  
  - dsControlFlow and similar default styles were added with KDE Frameworks 5, 
so kateversion="5.0" is required in the header
  - WordDetect for things like "end if" is not sensitive to multiple spaces 
anymore. Are you sure this is correct? If not, please keep RegExpr.
  
  Please provide an updated revision, I think the next one is good to go in. :-)

INLINE COMMENTS

> sql-mysql.xml:9
>  -->
> - section="Database" extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" 
> casesensitive="0" author="Shane Wright (m...@shanewright.co.uk)" license="">
> + section="Database" extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" 
> casesensitive="0" author="Shane Wright (m...@shanewright.co.uk)" license="">
>

kateversion="5.0" is required due to dsControlFlow.

> sql-oracle.xml:4
>  
> - section="Database" 
> extensions="*.sql;*.SQL;*.ddl;*.DDL;*.trg;*.TRG;*.spc;*.SPC;*.bdy;*.DBY" 
> mimetype="text/x-sql" casesensitive="0" author="Andrey Karepin 
> (egdf...@opensuse.org)" license="LGPL">
> + section="Database" 
> extensions="*.sql;*.SQL;*.ddl;*.DDL;*.trg;*.TRG;*.spc;*.SPC;*.bdy;*.DBY" 
> mimetype="text/x-sql" casesensitive="0" author="Andrey Karepin 
> (egdf...@opensuse.org)" license="LGPL">
>

Since you are using dsControlFlow and similar new itemDatas, you have to change 
kateversion="5.0".

> sql-oracle.xml:1873
> + beginRegion="if_block" insensitive="true"/>
> + endRegion="if_block" insensitive="true"/>
> + beginRegion="case_block" insensitive="true"/>

Hm, I can see that WordDetect is possibly faster than the RegExpr here. Still, 
I *assume* there are arbitrary many spaces allowed in "end if". If (and only 
if) this is true, then I would prefer to keep the RegExpr variant. Same for the 
others.

The changes from StringDetect to WordDetect are certainly fine.

> sql-postgresql.xml:6
>-->
> - section="Database" extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" 
> casesensitive="0" author="Shane Wright (m...@shanewright.co.uk)" license="">
> + section="Database" extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" 
> casesensitive="0" author="Shane Wright (m...@shanewright.co.uk)" license="">
>

kateversion="5.0" :-)

> sql.xml:8
>  
> - extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" casesensitive="0" 
> author="Yury Lebedev (yurylebe...@mail.ru)" license="LGPL">
> + extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" casesensitive="0" 
> author="Yury Lebedev (yurylebe...@mail.ru)" license="LGPL">
>

kateversion="5.0"

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, ngraham, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen added a comment.


  There is a bug on which I fell while completing the tests: the output is not 
stable.
  
  Several successive calls to `./bin/kate-syntax-highlighter 
$PROJ/autotests/input/test.sql` give a different result. `valgrind` returns a 
lot of "Conditional jump or move depends on uninitialised value(s) ", but I do 
not have the debug symbols and I do not know which library the error comes from 
(I tried with qt5-debug-base without result).

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11470

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, ngraham

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen updated this revision to Diff 29866.
jpoelen added a comment.


  File mode restored for autotests/input/highlight.sh and 
autotests/input/highlight.sh.syntax

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11470?vs=29865&id=29866

BRANCH
  sql

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, ngraham

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[jonathan poelen]

jpoelen created this revision.
jpoelen added reviewers: Framework: Syntax Highlighting, dhaumann.
Restricted Application added a project: Frameworks.
jpoelen requested review of this revision.

REVISION SUMMARY
  Previously, the variables prefixed by if / case / loop / end are a mix of 
keywords and normal text.
  
  Improvements:
  
  - Categorization of "Keywords" to "Operator Keyword" and "ControlFlow".
  - Removes duplicate keywords in "functions" and "keywords" list.
  - Some elements of the keywords list are moved to the functions list.
  - Adds SQL functions.
  - Removes symbols from Keyword list.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  sql

REVISION DETAIL
  https://phabricator.kde.org/D11470

AFFECTED FILES
  autotests/folding/test.sql.fold
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql.html
  autotests/html/test.sql_oracle.html
  autotests/input/highlight.sh
  autotests/input/highlight.sh.syntax
  autotests/input/test.sql
  autotests/input/test.sql_oracle
  autotests/reference/test.sql.ref
  autotests/reference/test.sql_oracle.ref
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: #frameworks, michaelh, ngraham