Configuration for formatter

[gavinking]

We need a way to configure the formatter.

• Config needs to be per-project.
• It needs to be persisted in a git-friendly form and needs to be editable as plain text.

So a properties file type thing would be a great start, though I suppose we will eventually want a GUI for it.

[lucaswerkmeister]

The formatter already reads the .ceylon/config file and uses settings from the “formatter” section (eclipse-archived/ceylon.formatter#32), so we just need to add a nice UI for it.

(You can also read other config files, so we could use an IDE-specific config… but I really don’t see the point in that.)

[akberc]

Approach:

• Project configuration with optional workspace settings (future?)
  
• Implement IPersistentPreferenceStore over Ceylon config, with scope on config file section formatting. The other option is to use the CeylonProjectConfig but that is geared towards the compiler and builder and not for individual option maps. Also, using an IPersistentPreferenceStore implementation allows us to store global config in Eclipse workspace preferences.
• Only store options that are changed from default values.

We will need to group the options into sections that show on different tabs.

[lucaswerkmeister]

You can also put global configuration into the user Ceylon config file - IIRC it cascades just like Git config: options from the project config file override options from the user config file.

In fact, AFAIK, you can put a config file on any directory level, so you could get a workspace-specific config in $workspace/.ceylon/config. But I haven't tested that.

(Sent from phone, sorry for the lack of formatting.)

[gavinking]

However you do it, I would like Eclipse to store its formatter prefs in some place where the commandline formatter can see them.

[quintesse]

In fact, AFAIK, you can put a config file on any directory level, so you could get a workspace-specific config in $workspace/.ceylon/config.

Well you could, but that would only work when your projects are subfolders (at some level) inside the workspace. If that's not the case the command line version would not know about it.

I do think that if both Eclipse and the CLI version need to use the same settings that the only real option we have is using our own config file. And in that case I also think that it should be the same format we already use for the configuration.

But my suggestion would be to just use a separate file within the Ceylon folder, let's say .ceylon/format or .ceylon/style.

I'm not certain how to deal with the workspace settings though. I think the only solution would be to always copy the workspace settings to the format config file and apply the project settings on top of them. The problem is to figure out afterwards which settings were made as part of the project and which were part of the workspace.

None of the things I can come up with will win any beauty prizes. We could either store them as a separate Eclipse specific section in the file, a section that gets ignored by the CLI tool. Or we could store them in a separate file .ceylon/format.eclipse. Or just use Eclipse's preferences settings.

[lucaswerkmeister]

Well you could, but that would only work when your projects are subfolders (at some level) inside the workspace. If that's not the case the command line version would not know about it.

Damn, you’re right, that’s not necessarily the case.

But my suggestion would be to just use a separate file within the Ceylon folder, let's say .ceylon/format or .ceylon/style.

Why not just a [formatter] section in the regular config file, as it’s already implemented? (Also, I can’t find how to pass a file name to CeylonConfig.get, but I might just be overlooking something.)

[gavinking]

Since all the infrastructure for the config file is already implemented, I don't see why it shouldn't go in there.

[FroMage]

Well, for configuration syncing there's a conflict of locations where it makes sense to sync:

• Project-local config
• User-global config (only Ceylon)
• Workspace-local config (only Eclipse)

Let's not pretend that the Ceylon CLI can read config from a workspace, this is not a notion that exists on the File System.

For project-specific settings there's no question. For the rest, what we can do is either:

• Just store the workspace settings in the workspace and the CLI will not be able to use it
• Store the workspace settings in the user config with a big fat warning and both can use it, but it's not workspace-specific

[gavinking]

• Store the workspace settings in the user config with a big fat warning and both can use it, but it's not workspace-specific

Honestly this sounds fine to me.

[quintesse]

Why not just a [formatter] section in the regular config file, as it’s already implemented?

First of all, there's almost no work involved, the code doesn't care from which file the information comes.

So why not in that file? Because to me the config file is mostly meant for required things. Now this is not a hard and fast rule but most of its settings are about things that if you don't set them things don't work,

The formatting on the other hand is completely personal, to be honest I do not want your blooming formatting settings in my project!

So a team of people might just say: "please add your format file to your .gitignore, we don't want one person's settings to conflict with somebody else's".

If we put them in the normal config file suddenly people can't even share their important setting s anymore.

That's why I think it should be in a separate file, at least.

[lucaswerkmeister]

Fine with me too. We can just call them "user-specific" and forget that they're workspace-specific in JDT :)

----- Ursprüngliche Nachricht -----
Von: "Gavin King" notifications@github.com
Gesendet: ‎17.‎07.‎2014 12:00
An: "ceylon/ceylon-ide-eclipse" ceylon-ide-eclipse@noreply.github.com
Cc: "Lucas Werkmeister" mail@lucaswerkmeister.de
Betreff: Re: [ceylon-ide-eclipse] Configuration for formatter (#987)

Store the workspace settings in the user config with a big fat warning and both can use it, but it's not workspace-specific
Honestly this sounds fine to me.
—
Reply to this email directly or view it on GitHub.

[FroMage]

I don't know about user-project specificity. It depends on several things:

• Does Eclipse store the JDT formatting rules in the generic .project file?
• Do projects store their Eclipse .project in their VCS?
• Do projects store their Ceylon .ceylon/config in their VCS?
• Do we want to introduce a convention of .ceylon/config being stored in VCS while .ceylon/user-config not being stored there?

IMO if you have a project formatting settings, it should apply to the whole project, and override any user setting for formatting.

[gavinking]

The formatting on the other hand is completely personal, to be honest I do not want your blooming formatting settings in my project!

I see how that's correct. There should be one canonical set of formatting standards for the project. The whole point of a formatter is to avoid "format wars".

[quintesse]

There should be one canonical set of formatting standards for the project

Let's just let people decide that for themselves, shall we?
If and when they want that they can do so and they can just add the "format" file to Git and things will work just fine, and if they don't want that they just leave it out.
But adding it to the "config" file takes away that choice.

[tombentley]

imo it's far more likely people will just add the whole .ceylon directory or not, as a whole. That's what I'd do with the eclipse .settings director, for example, because I have no idea if any of the files in there are interdependent. In other words, for that to really be a choice you need to be documenting the contents of that directory, so people know that adding or not an individual file will have the effect they intend without nasty side-effects.

[quintesse]

Well, I'm not sure if there are any nasty side effects, but yes it should be documented of course. We already do this btw : http://www.ceylon-lang.org/documentation/1.0/reference/tool/config/

Btw, another good reason IMHO to use separate files is that you can easily copy them around. You could have different "formatting profiles" lying around and the only thing you'd have to do to apply it to another project is to copy it. On top of that in the CLI tool you could easily point to a different one saying "format the code with this particular style".

[quintesse]

Btw, I just made some changes to the Config API that will make using a separate file even simpler.
Before the CeylonParser would have a bunch of loadConfigXxx() and findConfigXxx() methods and besides not being a proper place for those methods they were hard-coded to just look for a config file.
Now, you could load whatever file you want with the Config API, but the lookup code that handles finding and merging configuration files is useful in other cases as well, so I now split it out into a new class ConfigFinder and made it more configurable.
You can take a look at CeylonConfigFinder to see how easy it would be (a one-line change) to create one for a differently named file.

[lucaswerkmeister]

I’m inclined to agree with @quintesse. Even though I’ve previously argued that having user-specific formatting settings is evil, his arguments make a lot of sense :)

But then I remembered that the formatter also supports “include” configuration with include=fileName (both in config files and in command line arguments). With this, you could also create @quintesse’s workflow by adding formatter.include=.config/formatter to the config file. (Untested – not sure what the path needs to be relative to, especially from the IDE.)

This also allows you to keep different “formatting profiles” – e. g. in ~/.ceylon/formatter/ – and change which one you include on the fly. (And they can cascade! include=/etc/ceylon/companyWideFormatterBaseSettings or something like that.)

And now I’m not sure if it’s better to read from a specific config file by default.

@quintesse does the new Config API support something like “look in .ceylon/formatter, then .ceylon/config, then ~/.ceylon/formatter, then ~/.ceylon/config etc.”? (Or does that sound crazy to you? I’m really not sure how I feel about it.)

[quintesse]

And they can cascade!

I'd not use includes, they are also notoriously fragile across multiple OSes, you show include=/etc/ceylon/companyWideFormatterBaseSettings which for example won't work for our team where all 3 supported OSes are represented.

I too would think more along the lines of having "profiles", stored in ..../.ceylon/formatter/* which we could find using the same rules we use for finding ..../.ceylon/config (which means they would automatically be found in ~/.ceylon/ and /etc/ceylon or their alternatives on other OSes). We might even add an extra search path for the distribution folder so we could add default formatter profiles to the installation itself.

Then if you run the formatter without any profile name, and none its otherwise specified, it could look for default. Or you specify an explicit profile name and it will look for that file. (First in .ceylon/formatter/xxx, then up the hierarchy and finally trying ~/.ceylon/formatter/xxx, /etc/ceylon/formatter/xxx and $CEYLON_HOME/defaults/formatter/xxx)

@lucaswerkmeister right now the API supports only looking into a single file. Although that could possibly be changed. But yes, I think it's a bit crazy :)

If we'd go for this idea of profiles we could get rid of this .ceylon/format idea of mine because the configuration would be a single line, like formatter.profile=company-defaults.

Point is I'm still not sure how I feel about having it in .ceylon/config (but at least it would be very easy to change and having a special .ceylon/format file for a single line doesn't make sense), to me the whole idea is that there is a reason why formatters in IDEs have so many options and why nobody adds those preferences to Git. But right now I can't think of anything better than what I stated above.

[akberc]

Great points. To summarize:

• Formatting preferences are a user's preferences and may not be desirable at a project level
• At the moment, ceylon.formatter reads them the 'formatter' section in .ceylon/config
• A user's global preferences in file named 'style' would be very useful in ~/.ceylon
• Eclipse workspace storage in the workspace .metadata is not very useful.
• Ceylon config tool's Git-like hierarchy is preferred.

As such,

• will support current formatter functionality, reading and overriding, in order -
  • built-in,
  • user folder .ceylon/style file, formatter section,
  • project config file, formatter section
• would allow saving of profiles to arbitrary locations

Do we need to read or store a formatter version in the settings? Formatting options are done once in a while and then almost forgotten. Any changes to default built-in settings in the future will cause annoyance. With a settings version, it will give future versions of the formatter a chance to adapt current settings to new defaults.

[quintesse]

Do we need to read or store a formatter version in the settings

I think the best thing to do is to not leave things open to interpretation and always write all the formatting options to the file. So unlike preferences that are merged, styles would always be complete (which is also how Eclipse works, you don't just set single style options on a project, you either take the workspace settings or you choose a completely different style, even if that style differs by just a single setting).

That way if a new formatter is released with new defaults you will still have your old behavior. And any new settings will just get their default value because they would be missing from the style file.

[gavinking]

@akberc are you actively working on this? We plan a release in a month, and it would be nice to have this in. WDYT?

[lucaswerkmeister]

Apparently, for FormatterTabMisc, doCreatePreviewPane() isn’t called, so in doUpdatePreview() fPreview is still null.

[lucaswerkmeister]

Ah, no, it is actually related to inline annotations – because the preference is updated after creation, when a listener is registered, which then wants to update the preview, which doesn’t exist yet because we’re still initializing.

[lucaswerkmeister]

Okay, fixed as well. There’s still a small problem – if you check “all” (annotations) and reopen the config, it’s unchecked and the annotations string pref now contains [Ljava.lang.String;@abcdef, but at least it can be opened :)

[lucaswerkmeister]

Whew, that was a hairy fix, including a runtime subtyping check bug :( but now it works!

[akberc]

Yes, I was stuck on this last night.

Great stuff!! You take the lead and let me know what you want me to add or change. Update the sample code?

Unrelated to formatter, but I will add hooks to the new module wizard for author name and version number (variables just to fill in the Style tab that makes room for formatter tab under it..

[lucaswerkmeister]

Okay, I think I’m done for now! The only two remaining problems I have:

• The text field for the “new module author name” is very thin for some reason (perhaps OS-specific?) – only ~10 pixels high.
• CeylonStyle saves the used profile in style.formatterProfile in a separate file, while the CLI formatter tool uses the profile from formattool.profile in the default config file. I think the IDE should use the same option as the CLI.

[akberc]

How about we take out the main Style tab (with the author version) and the .ceylon/style file out. It was just a placeholder for a project properties page, and when we have some real style options, we can bring that back in if needed.

@lucaswerkmeister , @quintesse What should the nesting structure for project properties?

- Ceylon Formatter
- Ceylon Compiler
-- Build Paths
-- Module Repositories

-- OR --

- Ceylon
-- Compiler Build Paths
-- Module Repositories
-- Formatter Profiles

[lucaswerkmeister]

How about we take out the main Style tab

Yeah, that might be better for now.

What should the nesting structure for project properties?

Java / JDT has four toplevel property “pages”(?), but I don’t see why that would be an advantage… toplevel Ceylon seems more natural to me.

[quintesse]

I think @gavinking might have a stronger opinion on that than I do. Personally I'd probably copy Java's example, but I have no strong feelings about it.

[akberc]

I took the liberty of calling the top-level 'Ceylon Configuration' for that is literally what the four tabs do - update the Ceylon configuration for the project. Please change text in plugin.xml if needed.

I think we are ready to merge with master.

[akberc]

Bug: does not switch config value for profile back to default. Looking into it. But go ahead with merge.

[lucaswerkmeister]

@akberc I think I’ve found the bug:

public boolean performApply() {
    try {
        if (fFormatterProfileManager.getSelected() != null
                && fFormatterProfileManager.getSelected().getName() != "default"
                && fFormatterProfileManager.getSelected().getName() != "unnamed") {
            CeylonStyle.writeProfileToFile(fFormatterProfileManager
                    .getSelected(), project.getLocation().toFile());
            CeylonStyle.setFormatterProfile(project,
                    fFormatterProfileManager.getSelected().getName());
        }

You still need to set the formatter profile even if it’s the default. (But it’s probably correct that you don’t have to write it.) FormatterConfigurationBlock.java, around line 570.

[gavinking]

How about we take out the main Style tab

Yeah, that might be better for now.

Yes, please get rid of it. We don't want a page with one text field on it.

[akberc]

@lucaswerkmeister Bug fixed - and made the values constants for safety

[lucaswerkmeister]

Works great now, thanks a lot @akberc!

[lucaswerkmeister]

Done!! Thanks a lot @akberc for all your work here!

[akberc]

No worries. Thank you for testing, input and making the options variable! Sorry for being a bit late on this.

On another note, Github Windows client seem to have clobbered the plugin.xml git history on push, showing the whole file as replaced.

I will delete branch formatter-profiles - the original branch I was committing to.

[gavinking]

Alright, so thanks so much, @akberc and @lucaswerkmeister, this is overall actually really well done. I've cleaned the presentation of some of the options a little, but those were really very small changes.

Great work!