Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use config-file instead of constant/static variable in formatters for supported languages #33

Open
asccc opened this issue Jul 10, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@asccc
Copy link
Member

asccc commented Jul 10, 2020

Currently, all formatters use a static variable (or constant) to indicate support for specific languages.
This works, but has some performance penalties, because (in worst cases) all formatters are autoloaded at runtime just to check if a specific language is supported.

My suggestion would be some kind of mapping/config file.
Example:

formatters:
  - formatter: ClangCodeFormatter
    languages: 
      - c
      - cpp
      - csharp
      - java
      - objectivec
@asccc asccc changed the title Use language mapping instead of constant/static variable in formatters Use config-file instead of constant/static variable in formatters for supported languages Jul 10, 2020
@wookiefriseur
Copy link
Contributor

So you're thinking about a loader class that checks the config and then loads the formatter class on demand?
Is that something we would load into env at start?

Sounds good.

Some thoughts (not sure)

File Extensions
With that change we could think about adding file extension mappings to it as well

Formatter Names in the config
The formatter name probably should have a fully qualified class string (with namespace) in case we want to use external packages/plugins

Config file format
I'm split between putting it into a separate php/yaml or having the mappings inside the config handling class. The external file makes it more obvious that it's a config file, but the mapping is strongly coupled with the code formatter class names anyways. If it's all in the class, we don't have to load an extra file and the formatter class strings could be generated. But I don't know what's better regarding performance.

@jr-cologne jr-cologne added the enhancement New feature or request label Jul 11, 2020
@jr-cologne
Copy link
Member

I like your ideas. I don't know whether the performance boost would be significant, but it would indeed be a nice little improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants