search

eclipse-wildwebdeveloper / wildwebdeveloper

Simple and productive Web Development Tools in the Eclipse IDE
https://projects.eclipse.org/projects/tools.wildwebdeveloper
Eclipse Public License 2.0
190 stars 71 forks source link

Formatting XML removes blank lines. #1006

Closed garretwilson closed 1 year ago

garretwilson commented 1 year ago

I updated to Eclipse EE 2022-12 (Windows 10) and now suddenly Eclipse formats pom.xml files completely differently.

Specifically if I open pom.xml and hit Ctrl+Shift+F to format the XML file (as I have done for many years in Eclipse), suddenly, all of the blank lines are removed!!!

These blank lines were added to make the POM more readable. I can't just have blank lines ripped out of dozens of POMs. Besides a new version of Eclipse shouldn't radically change the formatting like this without giving us the option to turn it off.

How do I put the Eclipse POM XML formatter back to how it was in Eclipse 2022-09 and earlier (except for any bug fixes, of course)?

mickaelistria commented 1 year ago

The formatter is provided by LemMinX. Maybe Wild Web Developer misses to configure a few things? @angelozerr Are blank line supposed to be removed by default in LemMinX formatter?

angelozerr commented 1 year ago

Are blank line supposed to be removed by default in LemMinX formatter?

I think,

@garretwilson could you share your pom.xml file please? I add too @JessicaJHee who has worked on the new formatter. We plan to release LemMinx today or tomorrow.

garretwilson commented 1 year ago

Are blank line supposed to be removed by default in LemMinX formatter?

That is completely missing the point.

I don't care which formatting engine you use. But if Formatter Foo was used in Eclipse 2022-09, and you switch to Formatter Bar in Eclipse 2022-12, I expect you to have configured Formatter Bar so that by default it formats the XML the way it did in 2022-09.

How Formatter Bar operates "by default" is irrelevant. If you switched formatter engines, you should have configured the new engine to start out configured to format documents the way the old engine formatted them. If users want to benefit by new features of the new formatter engine, they can then make modifications to the configuration.

garretwilson commented 1 year ago

… could you share your pom.xml file please?

I'm not sure why you need a sample POM to see that blank lines are removed. Like:

<foo>
  <bar>example</bar>

  <bar>example</bar>
</foo>

becomes

<foo>
  <bar>example</bar>
  <bar>example</bar>

But here is a complete POM where you can see this: https://github.com/globalmentor/globalmentor-base/blob/main/pom.xml

garretwilson commented 1 year ago

Even if the developers didn't take the time to make Formatter Bar configured by default to format the way Formatter Foo was configured, at the very, very, very, very, very least there should be a configuration for the developer to go configure Formatter Bar to work the way Formatter Foo did.

So where is the configuration where I can turn off removal of blank lines?

If you did not provide a configuration for the developer to configure the new formatter to behave as the old one did, the old formatter should never have been replaced with the new one. That's just rule one of usability politeness.

garretwilson commented 1 year ago

We plan to release LemMinx today or tomorrow.

This confuses me. Are you saying that this new LemMinx was included in Eclipse 2022-12 (which was already released) before LemMinx itself was released??

akurtakov commented 1 year ago

I would like to remind you that these are volunteer driven projects and as such they provide whatever someone decided to spend his time on .

mickaelistria commented 1 year ago

So where is the configuration where I can turn off removal of blank lines?

nowhere as far as I know. Contributions to enable are and would be welcome.

garretwilson commented 1 year ago

I would like to remind you that these are volunteer driven projects and as such they provide whatever someone decided to spend his time on .

But even volunteer-driven projects should follow best practices. This isn't just any volunteer-driven project—Eclipse is part of the Eclipse Foundation, which is actually funded and has a set of rules for included code.

It sounds like you are saying that if someone is a volunteer that gives them an excuse not to use best practices and that the code doesn't need be reviewed.

akurtakov commented 1 year ago

But even volunteer-driven projects should follow best practices. This isn't just any volunteer-driven project—Eclipse is part of the Eclipse Foundation,

Sorry for bursting the bubble but most of the Eclipse Foundation projects are not funded in any way but by people and companies deciding to work on what they care for.

which is actually funded and has a set of rules for included code.

And which one of these rules exactly implies that a project can't change it's runtime behavior?

It sounds like you are saying that if someone is a volunteer that gives them an excuse not to use best practices and that the code doesn't need be reviewed.

I'm saying that if someone cares for smth he sends a PR .

garretwilson commented 1 year ago

… most of the Eclipse Foundation projects are not funded in any way … And which one of these rules exactly implies that a project can't change it's runtime behavior?

My point is that Eclipse is a professional, collaborative project used across enterprises, not just a single person's weekend proof-of-concept. Volunteer code should be using best practices and should be reviewed by the teams.

In this case, if the old formatter engine is being replaced with a new formatter (which conceptually is perfectly reasonable) it should in no case remove functionality without giving the user a choice. If the volunteers don't have time to provide feature parity, fine—but in that case the old formatter engine should be left in or provided as an option.

akurtakov commented 1 year ago

but in that case the old formatter engine should be left in or provided as an option.

^^ requires time spent which no one seems to have found time for . So let's stop with the lectures what should be done as there is single constraint that defines what/how/why happens in a project and this is hours spent by people developing project. Thus everything like should/must/never/keep/etc. is plain functions of the time spent on the project and thus all these words become meaningless unless enough people start helping out.

mickaelistria commented 1 year ago

@garretwilson You seem to have false impression that Eclipse has a contract of stability of functionality towards its user and yourself. There is no such thing, like it or not; Eclipse projects are driven by its committers who are free to do together what's best according to their vision. If you want to contract sone guarantees regarding some features that are vital to you, you'll need to find some 3rd party developer OK to support your wishes against a fair amount of money. Good luck with that!

garretwilson commented 1 year ago

You seem to have false impression that Eclipse has a contract of stability of functionality towards its user and yourself.

No, it's not that, honestly. I think I'm from an older school where developers used to have a certain contract "with themselves" to produce good work. There's a culture nowadays where "agile" and "devops" principles are distorted to encourage throwing anything together with no documentation with little regard to user experience or good practices.

In this case I wonder if the team actually considered feature parity with the formatting engines and whether there should be an abrupt change in user experience; and made a tradeoff analysis that was documented? Did the change in the formatter behavior get listed in the Eclipse changelog? Again the excuse "but we're just volunteers; we don't have time to do tradeoff analyses or to document our decisions" irritates me; I do tradeoff analysis and document out of pride of my work and discipline, even for a project I'm working on all by myself.

I realize that not everyone is me. The best practices in this case might not have been evident to everyone, and I should have taken a softer approach at explaining them; I'll try to do better next time.

In any case I've reported the problem and explained the best practices. Maybe one of the volunteers will find them useful with future work.

akurtakov commented 1 year ago

The whole "agile"/"devops" bites yes but it's in entirely different direction - 10 years ago there was team of 5-6 people per component. Nowadays, a team of 4 is responsible for 16 projects . I think that this explains everything.

mickaelistria commented 1 year ago

I think I'm from an older school where developers used to have a certain contract "with themselves" to produce good work

We'd definitely benefit from more contributors having this culture. Please consider contributing PRs to the project until you eventually get nominated as a committer.

mickaelistria commented 1 year ago

Please try latest snapshots of Wild Web Developer, which are supposed to have it fixed thanks to #1007

garretwilson commented 1 year ago

Please try latest snapshots of Wild Web Developer …

I'm trying to download the latest snapshot from the WWD p2 snapshot site. It shows there's a Mylyn WikiText update and a WWD update. I only want the WWD update, but there is no option to select one or the other.

Apparently someone neglected to sign the Mylyn OSGI bundles for 3.0.44.202211082139. I prefer not to install unsigned plugins (at least when I download Eclipse I can compare the hashes manually), and the Mylyn update isn't even what I'm looking for here to fix the XML formatting bug.

So how can I install just the new version of WWD without installing the unsigned Mylyn plugin update?

angelozerr commented 1 year ago

@mickaelistria it will not work, it will require the PR https://github.com/eclipse/wildwebdeveloper/pull/1009 which provides the Preserve new lines set to 2by default. The new formatter manages preserve new lines with a number, sothe usecase of @garretwilson will work, but it is not possible to preserve any new lines. If anybody requires this capability, please create an issue at https://github.com/eclipse/lemminx

@garretwilson I have played with your pom.xml with the new formatter and I have the same result than WTP.

I understand your frustration. WTP XML Editor works very good and when you install new Eclipse IDE, you expect having bugs fix or some improvement but not regresision. We have already a regression with https://github.com/eclipse/wildwebdeveloper/issues/872, it will take some time because it requires some improvement on Platform Text but it will open the door to support .editorconfig.

For XML support, they will have 2 choices :

We did the second choice and we have a lot of feedback, contribution not only from Eclipse User but from another community like vscode, sublime, etc

XML Language Server tries release by release to have the same level than WTP XML Editor and provides even some more features than WTP XML Editor like inline rename tag, codelens to associate easily an XML to a Schema, RelaxNG support etc. We had had several problem with XML Catalog with WTP, indeed it was annoying but we fixed it. We try to be the most responsive, by fixing bugs and create release ASAP.

For the formatter topic, we study several XML formatter coming from Eclipse IDE with WTP, IJ and Oxygen and we try to support the 3 formatters behavior with a combinaison of settings.

On LemMinx side we wrote a lot of test with formatter for each settings and combinaison of settings, so if we found some regression we will able to fix it without breaking the existing.

In other words, please create any issues on LemMinx https://github.com/eclipse/lemminx if you find some regression, or you want some improvements. Our goal is to provide the same level than WTP XML Editor and more. Thanks!

garretwilson commented 1 year ago

it will not work, it will require the PR https://github.com/eclipse/wildwebdeveloper/pull/1009 which provides the Preserve new lines set to 2by default.

So since this is not fixed in WWD, would it not be appropriate to change the status of this ticket back to "open"? It seems it was inappropriately closed if it isn't fixed, not even in a snapshot version.

(Maybe I misunderstood @angelozerr's comment, but he seems to be saying that this ticket is not fixed until PR #1009 is accepted.)

garretwilson commented 1 year ago

move to LemMinx the XML Language Server which is now integrated in many IDE/Editor like vscode, sublime, vim, etc and Eclipse IDE.

It good that you considered options and it is reasonable that you decided to switch to a formatting engine that is used across platforms and is (hopefully) supported by a large community.

The problem here is 1) not providing feature parity (when the old formatter honestly didn't have very many formatting options to begin with) and 2) not providing a way to customize the changed settings.

Since you say that you looked at other formatters, you no doubt looked at js-beautify, which is now the default formatter in VS Code for many formats including HTML. (Over the years I have provided a lot of input and even paid real money bounties to bring the js-beautify HTML5 support in line with the specifications, e.g. in beautify-web/js-beautify#1033 and Glavin001/atom-beautify#2210.) Looking at the formatting options for js-beautify will provide valuable suggestions for which formatting options you should expose to the user.

In my opinion the following options are so important that they should be exposed at a minimum:

The last option is pretty important. I believe an older version of WWD would strip ending newlines (although I don't remember for sure), which would make the XML inconsistent in projects that followed the traditional POSIX definition of a text line.

angelozerr commented 1 year ago

1) not providing feature parity

It is very hard to do that because our formatter follows the same strategy than Oxygen, see https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md#formatting-strategy As we have so many requirement to have this formatting strategy we decide to implement it, but WP doesn't follows those rules.

Perhaps we should provide an option to disable this formatting strategy, we will see. We try to pick the best idea from each formatter that we found, but it is difficult to reproduce exactly the same behavior than WTP which follows different rules.

If you want to know supported settings of LemMinx, see https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md

angelozerr commented 1 year ago

@garretwilson for your information, I have found some tests with WTP XML Editor formatter. I have included it. Almost tests failed because mixed content format doesn't follow the same strategy than Oxygen and IJ that we implemented, but we will see if we could try to mimic WTP mixed content format.

See https://github.com/eclipse/lemminx/pull/1415#issuecomment-1354370337

It is shame that WTP XML doesn't provides tests without mixed content because it seems we have the same format that WTP (ex : if you format a pom.xml with lemMinx , it should be the same result than WTP)