spotless icon indicating copy to clipboard operation
spotless copied to clipboard

Add support for JSP/JSF in eclipse WTP

Open vbrandl opened this issue 4 years ago • 3 comments

When formatting some JSF inside a style attribute, the formatter destroys the JSF by stripping the closing }.

This happens on Gradle 6.4.1 with the plugin version 5.0.0 as well as with Gradle 6.0.1 and plugin version 4.3.0.

Here is a minimal example to reproduce the issue: https://github.com/vbrandl/spotless-formatting-jsf-reprod

After running ./gradlew spotlessApply in the repository, the diff looks like this:

   <div
-    style="width: 30px; height: 20px; background: #{someControllerName.someFunctionToGetTheColor(x)}" />
+    style="width: 30px; height: 20px; background: #{someControllerName.someFunctionToGetTheColor(x)" />

The following configuration is used:

spotless {
  format 'xml', {
    // ratchetFrom 'origin/master'
    target '**/*.xhtml'
    eclipseWtp('html')
    indentWithSpaces(2)
    endWithNewline()
    trimTrailingWhitespace()
    encoding 'UTF-8'
  }
}

vbrandl avatar Jul 14 '20 08:07 vbrandl

I can confirm the problem. I integrated your example quickly into a unit test for HTML formatting. But I have the same problem when I run the formatting in the Eclipse IDE using a HTML formatter. When I use the JSF/JSP formatter, it works fine. I am afraid I have not supported all WTP extensions. A list of the supported/tested once is in this README section.

Looking at the code, it seems that in the end the JSP/JSF formatting is based on HTTP formatter. This also corresponds to Eclipse documentation.

I suspect that Eclipse Webtools-JSF has a feature which provides some JSP/JSF format tweaks, but not sure. Anyhow, I am afraid this is a feature request, and Spotless Eclipse WTP would need to provide a JSP/JSF formatter type (like it now provides HTML, XML, ...).

fvgh avatar Jul 14 '20 19:07 fvgh

@vbrandl I am afraid I personally have right at the moment no time to provide an extension. Should not be that difficult, but some digging in the Eclipse code/config is required. So if you want to provide something, I can give you a quick tour through the spotless-eclipe-wtp code.

fvgh avatar Jul 14 '20 19:07 fvgh

I have the same problem, and some more info: the problems seems specifically related to the usage of EL expressions inside a style="" attribute.

style="display: #{dashboard.postIt == null ? 'none': 'block'};"> gets formatted as style="display: #{dashboard.postIt== null? 'none': 'block'">.

But when I change style="" by something else as a test: title="display: #{dashboard.postIt == null ? 'none' : 'block'};"> it is left untouched.

So I suspect the problem might be related to formatting of embedded CSS?

jpraet avatar Mar 23 '21 11:03 jpraet

Of course there is an issue with that, but... I've got o huge project with JSF and Spotless. We use WTP and there are really a few issues on thousands of pages.

I'd never use this kind of inline CSS. In any CSS framework there will be classes like none or display-none or something like this. We have or own displayNone. And if you change:

<div style="display: #{dashboard.postIt== null? 'none': 'block'">

to

<div class="display#{dashboard.postIt == null ? 'None': 'Block'}">

eclipse WTP formatter won't protest.

The only main issue with WTP and JSF we have is that it forces <script> tag to have type="application/javascript" even though it's not needed in HTML 5. But importing JavaScript formatter is a must for embedded JavaScript in XHTML files.

Emkas avatar Jan 11 '23 11:01 Emkas

You can always modify the behavior by following the eclipse step with a replace 'type="application/javascript"', ''. In ~ a week I'll have something to show which will hopefully make maintaining all the eclipse steps a lot easier.

nedtwigg avatar Jan 11 '23 22:01 nedtwigg

You can always modify the behavior by following the eclipse step with a replace 'type="application/javascript"', ''. In ~ a week I'll have something to show which will hopefully make maintaining all the eclipse steps a lot easier.

@nedtwigg, Thanks! After some tries it worked! Since maven properties are trimmed I had to go with:

<replace>
	<name>Remove unnecessary type of script in HTML 5</name>
	<search>script type="text/javascript"</search>
	<replacement>script</replacement>
</replace>

BTW. In the time you wrote this, a replacement in the replace could not be empty (this change was released 3 days after :smiley:), so I also had to update my maven plugin.

Emkas avatar Mar 07 '23 14:03 Emkas

The broader issue of support JSP/JSF in WTP is possible by finishing #1622.

nedtwigg avatar Mar 13 '23 18:03 nedtwigg