eclipse-archived / ceylon.formatter

A formatter for the Ceylon programming language, written in Ceylon.
Apache License 2.0
14 stars 11 forks source link

Line break after \i-escaped annotations #144

Closed lucaswerkmeister closed 7 years ago

lucaswerkmeister commented 7 years ago

The test for #102 currently fails – instead of

\ishared String issue102 = "PIDENTIFIER and AIDENTIFIER";

the formatter produces

\ishared
String issue102 = "PIDENTIFIER and AIDENTIFIER";

This only happens for \i annotations. Not sure why…

lucaswerkmeister commented 7 years ago

Hm. The text of \ifoo tokens used to be just foo (the grammar rewrote it), and we had to check the token length to recognize that it used to be \ifoo and resurrect the \i. But it looks like the token text is now actually \ifoo? @gavinking do you recall changing this?

lucaswerkmeister commented 7 years ago

Ah, that would be ceylon/ceylon@4c45a73508291578d07762bf566e2da9e4c0ce89 :)

gavinking commented 7 years ago

Ah, that would be ceylon/ceylon@4c45a73 :)

I came here to say that :)

lucaswerkmeister commented 7 years ago

Well, if you want to “make stuff easier for downstream tools”, perhaps tell them about it? :P

So I guess this means I can throw out most of visitIdentifier, and visitAnnotation needs to become a bit more clever in order to throw out the \i prefix.

bjansen commented 7 years ago

@lucaswerkmeister did you make any progress on this issue?

I temporarily disabled the tests on Jenkins because they were preventing the IDEs from building correctly.

FTR, clean publish compile-test test-formatter-tap-quick test-cli-tap-quick ide-quick is now simply clean publish ide-quick. I'll put the whole command back once this issue is fixed.

lucaswerkmeister commented 7 years ago

Been busy lately… but it shouldn’t be a hard fix.

But why did you need to disable the tests on Jenkins? That shouldn’t be necessary IMO – there should be a ceylon.formatter build with tests, and the IDE builds should be separate without needing to run the ceylon.formatter tests.

bjansen commented 7 years ago

Well there is a ceylon.formatter build with tests, but because the tests are failing it does not trigger the build on the other projects.

lucaswerkmeister commented 7 years ago

@bjansen job 542 failed, and I suspect it’s just the old TAP file still laying around in the workspace. Since I’ve fixed the build, can you reenable the tests?

lucaswerkmeister commented 7 years ago

(Well, 71c9d1d1b0e4cad0356b374e99223ebc3bf8c9e1 should also fix the build, but still, might as well run the tests again.)

bjansen commented 7 years ago

Done, it's back to blue now. You're right, I think the clean goal does not remove TAP results because they are at the root of the project structure.