include php blade plugin in php cluster

[haidubogdan]

Laravel is one of the most used frameworks in php, yet it still doesn't have support in Netbeans. (#7531 , #7231). Mostly the main missing support is for blade templates syntax.

I've started to work on a plugin https://github.com/haidubogdan/netbeans-php-blade-plugin 3 years ago. After using antlr as a lexer and parser, I found that scaling the plugin was much maintainable. It's not the cleanest code, but I realized that I will always reach to the 99% finish status if I don't do the first pull request.

TODO

• [x] Submit an ICLA (sent on 3 august) //I didn't received any response but I can consider it was submited
• [x] Change to the full name in commits
• [x] Check the license of the image (no response received from Tailor [laravel], using personal icon)
• [x] Add unit tests
  • [x] basic lexer tests
  • [x] basic parser error tests
  • [x] basic completion tests
  • [x] basic navigator tests
  • [x] basic embedding tests (html, php)
  • [x] basic braces tests
  • [x] basic typehook tests
  • [x] basic formatting test (wip)
• [x] Write all features with screenshots

Main features

• Blade syntax coloring

• Declaration finder for
  • php elements
  • included paths
  • yield ids (section)
  • stack elements
• Php code completion inside expressions, and echo tags

• Brace matcher and folding

• Find template usage (only in the blade files context)

Custom directives

Project -> Properties -> Laravel Blade -> Custom Directives

Just add the php file where you added the custom directive implementation as in https://laravel.com/docs/10.x/blade#extending-blade.

Views folder

If you use blade templates outside of the generic laravel framework or have custom templates folders you can configure them for a project.

This will help the yield and view path completion

Global declaration finder for views paths

Possiblity to go to declaration finder for string parameters inside render, make, view methods.

Reformat and indentation

Experimental formatting and indenting

blade components

• Limited completion and declaration finder

  

• Config to set paths of the components class implementation for autocomplete & declaration finder.

[junichi11]

First of all, thank you for your contribution!

I hope to reach nb 23 deployment

Unfortunately, it's too late. The feature freeze date is July 26th.

Please write all features of this module with screenshots here as well. (not only the link)

Please add unit tests for features. e.g. code completion, indexer, navigator, parser, lexer, formatter etc. (also see: CslTestBase.java)

Did you submit an ICLA?

Probably, it takes a lot of time to review this.

[haidubogdan]

Hi, thank you for your feedback. I will prepare the unit tests. ( I hope the base parser unit test classes will be available now that it's included in the IDE folder). I didn't submit the ICLA. I will read about it.

[junichi11]

How did you generate icons? (Are there icons based on something?)

https://github.com/apache/netbeans/pull/7618/files#diff-24c53d9c7defe885e1ee8adee9ff009c57d69e75621ad84625d8f9eaa8d7c9e1

[haidubogdan]

How did you generate icons? (Are there icons based on something?)

The Laravel logo comes from laravel itself : https://laravel.com/docs/10.x/releases

The rest of the logos are manual done. With a contribution from erikn69 https://github.com/haidubogdan/netbeans-php-blade-plugin/commit/af56ce45b1fb27b58965ba77bc195bca1a8b76a2

[junichi11]

The Laravel logo comes from laravel itself

Please confirm the license of original logo. Thanks!

The rest of the logos are manual done. With a contribution from erikn69

This case is no problem. (because the license of the original icons are the Apache License.)

Please add a licenseinfo.xml (see: licenseinfo.xml)

[haidubogdan]

Please confirm the license of original logo. Thanks!

According to a discussion https://laracasts.com/discuss/channels/general-discussion/use-of-the-laravel-logo

If you use their laravel logo for commercial purpose, you need to have a partnership. So I think we should be ok here. But I will see how I can contact them for a safe trademark usage.

[junichi11]

Can you also ask for the license of the logo image itself? If we cannot clarify, we cannot add that image.

[mbien]

https://github.com/apache/netbeans/pull/7618.patch please make sure the commits have a full name in the header before merging

[haidubogdan]

Can you also ask for the license of the logo image itself? If we cannot clarify, we cannot add that image.

Sure, discussion in progress.

---

Having all these code updates to handle + the incomplete name in commit message, it is best that I close this pull request, an do a new one when the evolution is better prepared.

Thank you for your help and reviews.

[mbien]

Having all these code updates to handle + the incomplete name in commit message, it is best that I close this pull request, an do a new one when the evolution is better prepared.

this is rarely needed. It makes it also difficult for reviewers to see what changed between the PRs. You can simply add changes here and once the review is done, the last step would be to squash the commits before integration.

[haidubogdan]

Having all these code updates to handle + the incomplete name in commit message, it is best that I close this pull request, an do a new one when the evolution is better prepared.

this is rarely needed. It makes it also difficult for reviewers to see what changed between the PRs. You can simply add changes here and once the review is done, the last step would be to squash the commits before integration.

Ok, reopened. I wasn't sure on the best methodology.

[tmysik]

@haidubogdan thanks a lot for this PR, you did an amazing job when one sees what you were able to implement.

Please, resolve every single comment or feel free to provide your reasons why the given comment is wrong/not required :)

As @junichi11 mentioned, it would be great to have screenshots of the plugin features (they could be then reused in the News).

Thank you.

[tmysik]

@haidubogdan Just 2 comments:

1. please, do not be surprised, the number of comments cannot be small because your change is huge. so, be patient with us :)
2. once merged, are you going to maintain the plugin in the future? I mean, if there is a bug report, will you be able to fix it? Or add a new feature? I am asking since this is very important, our resources are limited, unfortunately, and in the worst case, if the plugin had many issues/problem, we would be forced to remove it, sorry

Thank you for understanding.

CC @junichi11

[haidubogdan]

@haidubogdan Just 2 comments:

1. please, do not be surprised, the number of comments cannot be small because your change is huge. so, be patient with us :)
2. once merged, are you going to maintain the plugin in the future? I mean, if there is a bug report, will you be able to fix it? Or add a new feature? I am asking since this is very important, our resources are limited, unfortunately, and in the worst case, if the plugin had many issues/problem, we would be forced to remove it, sorry

Thank you for understanding.

CC @junichi11

1. So far, I like the community, no problems here :) . As I do most of the implementation in my free time, it won't be that fast, but I hope to manage it in a week or two.
2. Well this would be my intention :) , depending on how much free time I have.

[tmysik]

@haidubogdan Cool then. No rush, take your time, you did really a lot of work.

@junichi11 This is the biggest contribution for PHP area in the Apache era, no? If I don't count you, of course 😁

[junichi11]

@haidubogdan Please don't rebase this branch onto the master branch or merge the master until the review is finished because it would be hard to review. Thanks.

[haidubogdan]

@haidubogdan Please don't rebase this branch onto the master branch or merge the master until the review is finished because it would be hard to review. Thanks.

Ok, good to know. I did one last rebase to correct the commit name

[junichi11]

Please check whether we should translate yourself once for strings of all files. (add // NOI18N or @Messages) Please check whether annotations are needed. (e.g. @CheckForNull, @NullAllowed,...)

[haidubogdan]

Blade Module features

[wip]

• Blade syntax coloring

• Declaration finder for
  • php elements
  • included paths (@include)
  • yield ids (@section)
  • stack elements (@push)

• Brace matcher and folding

• Find template usage (only in the blade files context)

---

• Custom directives

  Project -> Properties -> Laravel Blade -> Custom Directives

  Just add the php file where you added the custom directive implementation as in https://laravel.com/docs/10.x/blade#extending-blade.

• Views folder

  If you use blade templates outside of the generic laravel framework or have custom templates folders you can configure them for a project.

  This will help the yield and view path completion

• Global declaration finder for views paths

  Possiblity to go to declaration finder for string parameters inside render, make, view methods.

• Reformat and indentation

  formatting and indenting

• Blade components

  Limited completion and declaration finder

[mbien]

fyi BladeBracesMatcherTest failed the last two times CI ran on this PR

[haidubogdan]

fyi BladeBracesMatcherTest failed the last two times CI ran on this PR

Yes, unix vs windows naming difference for the goldenfile.

Unix searches for testUnclosedSectionDirective_01.testUnclosedSectionDirective_01.braces but the existing windows created file is named testUnClosedSectionDirective_01.testUnClosedSectionDirective_01.braces

[mbien]

the test case is called testUnclosed* while the golden file itself is called testUnClosed* which looks like a typo?

[mbien]

congrats! php tests are green :)

[matthiasblaesing]

This currently fails to build from source. With this patch I can build again:

diff --git a/php/php.blade/build.xml b/php/php.blade/build.xml
index 409c84c8400b..4ea7b71c6021 100644
--- a/php/php.blade/build.xml
+++ b/php/php.blade/build.xml
@@ -26,23 +26,35 @@
         <property name="v10.outdir" location="${src.dir}/org/netbeans/modules/php/blade/syntax/antlr4/v10"/>
         <property name="formatter.outdir" location="${src.dir}/org/netbeans/modules/php/blade/syntax/antlr4/formatter"/>
 
-        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}">
+        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}"  failonerror="true">
             <arg value="-o"/>
             <arg value="${v10.outdir}"/>
             <arg value="BladeAntlrLexer.g4"/>
             <arg value="BladeAntlrParser.g4"/>
+            <classpath>
+                <fileset dir="../../ide/libs.antlr4.runtime/external" includes="*.jar" />
+                <fileset dir="../../ide/libs.antlr3.runtime/external" includes="antlr-runtime-*.jar" />
+            </classpath>
         </java>
-        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}">
+        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}" failonerror="true">
             <arg value="-o"/>
             <arg value="${v10.outdir}"/>
             <arg value="BladeAntlrColoringLexer.g4"/>
+            <classpath>
+                <fileset dir="../../ide/libs.antlr4.runtime/external" includes="*.jar" />
+                <fileset dir="../../ide/libs.antlr3.runtime/external" includes="antlr-runtime-*.jar" />
+            </classpath>
         </java>
 
-        <java classname="org.antlr.v4.Tool" fork="true" dir="${formatter.outdir}">
+        <java classname="org.antlr.v4.Tool" fork="true" dir="${formatter.outdir}" failonerror="true">
             <arg value="-o"/>
             <arg value="${formatter.outdir}"/>
             <arg value="BladeAntlrFormatterLexer.g4"/>
             <arg value="BladeAntlrFormatterParser.g4"/>
+            <classpath>
+                <fileset dir="../../ide/libs.antlr4.runtime/external" includes="*.jar" />
+                <fileset dir="../../ide/libs.antlr3.runtime/external" includes="antlr-runtime-*.jar" />
+            </classpath>
         </java>
         <delete dir="${v10.outdir}" includes="*.tokens"/>
         <delete dir="${v10.outdir}" includes="*.interp"/>

[haidubogdan]

This currently fails to build from source. With this patch I can build again:

diff --git a/php/php.blade/build.xml b/php/php.blade/build.xml
index 409c84c8400b..4ea7b71c6021 100644
--- a/php/php.blade/build.xml
+++ b/php/php.blade/build.xml
@@ -26,23 +26,35 @@
         <property name="v10.outdir" location="${src.dir}/org/netbeans/modules/php/blade/syntax/antlr4/v10"/>
         <property name="formatter.outdir" location="${src.dir}/org/netbeans/modules/php/blade/syntax/antlr4/formatter"/>
 
-        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}">
+        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}"  failonerror="true">
             <arg value="-o"/>
             <arg value="${v10.outdir}"/>
             <arg value="BladeAntlrLexer.g4"/>
             <arg value="BladeAntlrParser.g4"/>
+            <classpath>
+                <fileset dir="../../ide/libs.antlr4.runtime/external" includes="*.jar" />
+                <fileset dir="../../ide/libs.antlr3.runtime/external" includes="antlr-runtime-*.jar" />
+            </classpath>
         </java>
-        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}">
+        <java classname="org.antlr.v4.Tool"  fork="true" dir="${v10.outdir}" failonerror="true">
             <arg value="-o"/>
             <arg value="${v10.outdir}"/>
             <arg value="BladeAntlrColoringLexer.g4"/>
+            <classpath>
+                <fileset dir="../../ide/libs.antlr4.runtime/external" includes="*.jar" />
+                <fileset dir="../../ide/libs.antlr3.runtime/external" includes="antlr-runtime-*.jar" />
+            </classpath>
         </java>
 
-        <java classname="org.antlr.v4.Tool" fork="true" dir="${formatter.outdir}">
+        <java classname="org.antlr.v4.Tool" fork="true" dir="${formatter.outdir}" failonerror="true">
             <arg value="-o"/>
             <arg value="${formatter.outdir}"/>
             <arg value="BladeAntlrFormatterLexer.g4"/>
             <arg value="BladeAntlrFormatterParser.g4"/>
+            <classpath>
+                <fileset dir="../../ide/libs.antlr4.runtime/external" includes="*.jar" />
+                <fileset dir="../../ide/libs.antlr3.runtime/external" includes="antlr-runtime-*.jar" />
+            </classpath>
         </java>
         <delete dir="${v10.outdir}" includes="*.tokens"/>
         <delete dir="${v10.outdir}" includes="*.interp"/>

I can add the updates.

I guess it could be something similar to https://github.com/apache/netbeans/blob/master/webcommon/javascript2.json/build.xml

[haidubogdan]

Hi, A progress update. I've started to work on some big code refactoring (hopefully improvement). This might take a week or two to finish.

The changes started from way the parser data collection works, so all the important functionality : code completion, declaration finder ... etc needs to be adapted.

[neilcsmith-net]

Removing milestone. Please retarget as required.

[tmysik]

@haidubogdan, I am OK with this PR but first, @junichi11 has to approve it since including a new module in the NetBeans build means responsibility of maintaining it too, so Junichi is the main person to decide (just to be clear - I don't think that Junichi should maintain it in the future, it should be @haidubogdan, ideally :smile:). Of course, any module can be removed later but let's think about it first so we can prevent it.

@haidubogdan, thank you for your work!

[haidubogdan]

@haidubogdan, I am OK with this PR but first, @junichi11 has to approve it since including a new module in the NetBeans build means responsibility of maintaining it too, so Junichi is the main person to decide (just to be clear - I don't think that Junichi should maintain it in the future, it should be @haidubogdan, ideally 😄). Of course, any module can be removed later but let's think about it first so we can prevent it.

@haidubogdan, thank you for your work!

@tmysik Thank you. While waiting for the review the only change I might add is the support for the new blade directives of Laravel 12.

[tmysik]

@haidubogdan sounds good to me. If it does not cause major changes to this PR :)