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

.clang-format is used instead of .unibeautifyrc #29

Open
lassik opened this issue Jul 18, 2018 · 10 comments
Open

.clang-format is used instead of .unibeautifyrc #29

lassik opened this issue Jul 18, 2018 · 10 comments
Assignees
Labels
question Further information is requested

Comments

@lassik
Copy link
Contributor

lassik commented Jul 18, 2018

With the current version from NPM (@unibeautify/[email protected]), the external program clang-format gets its settings from a file named .clang-format in the current directory, just as it does when it runs standalone.

I think .unibeautifyrc is also ignored altogether. I did a quick test with a .unibeautifyrc in the same directory as test.c (an example file to format) and it ignored .unibeautifyrc even when there is no .clang-format file in the directory.

This code: https://github.com/Unibeautify/beautifier-clang-format/blob/master/src/index.ts#L136
seems to fallback to -style=file as a default. I believe we should never use -style=file or .clang-format with Unibeautify - @Glavin001 I recall you talked about this in some GitHub issue under some project, giving reasons?

EDIT: Just to recap, here's the -style= part of clang-format --help:

  -style=<string>           - Coding style, currently supports:
                                LLVM, Google, Chromium, Mozilla, WebKit.
                              Use -style=file to load style configuration from
                              .clang-format file located in one of the parent
                              directories of the source file (or current
                              directory for stdin).
                              Use -style="{key: value, ...}" to set specific
                              parameters, e.g.:
                                -style="{BasedOnStyle: llvm, IndentWidth: 8}"
@stevenzeck
Copy link
Contributor

Can you share what your .unibeautifyrc file looks like?

@lassik
Copy link
Contributor Author

lassik commented Jul 18, 2018

Sure :)

$ cat .unibeautifyrc.yml
---
C:
  indent_size: 4
  indent_char: " "

@stevenzeck
Copy link
Contributor

Actually nothing from unibeautifyrc, except maybe preferBeautifierConfig (I'd have to test that), is used by clang-format, as you'll see there are no options implemented in this beautifier.

There are a lot of options for clang-format and we didn't have the time to go through them, add to Unibeautify, and transform them.

@lassik
Copy link
Contributor Author

lassik commented Jul 18, 2018

Aha, sorry I didn't read the code more thoroughly :) No wonder. Yeah, it probably has more options than anything else.

I couldn't find anything about preferBeautifierConfig on Google or GitHub. Is there a plan to have an option that allows people to ignore Unibeautify's settings in favor of the formatter's native ones?

@stevenzeck
Copy link
Contributor

Yep that's what prefer_beautifier_config does. Sorry it had underscores.

@Glavin001
Copy link
Member

@lassik Please see https://unibeautify.com/docs/beautifier-clangformat for documentation related to ClangFormat. In the Advanced section the prefer_beautifier_config is referenced as well no options are shown below. Thanks @szeck87 for your assistance above.

For details about the C language support see https://unibeautify.com/docs/language-c
Notice that indent_size and indent_char are not supported for language C so your .unibeautifyrc.yml file above will not work.


@szeck87 I think we should start including a link to the Website Docs in the repositories' README.md to mitigate questions like this.

Also, we Unibeautify editor extensions should display warnings when unsupported options are used for languages. Such as indent_size for C when ClangFormat (only beautifier for C) does not support indent_size (or any other option for that matter).

@Glavin001 Glavin001 added the question Further information is requested label Jul 18, 2018
@lassik
Copy link
Contributor Author

lassik commented Jul 18, 2018

Thank you for the pointers and sorry to raise ignorant concerns. I recalled from some earlier issue that the plan was to always use Unibeautify's unified formatting options instead of beautifiers' native options, so I didn't think to look for a prefer_beautifier_config option. But that issue was probably opened a long time ago (I can no longer find it). From that standpoint, I thought indent_size is such a common concern for C that it didn't occur to me that it isn't supported.

Also, we Unibeautify editor extensions should display warnings when unsupported options are used for languages. Such as indent_size for C when ClangFormat (only beautifier for C) does not support indent_size (or any other option for that matter).

Excellent suggestion. Could we make the CLI handle this? (Related discussion in Unibeautify/cli#42)

I think we should start including a link to the Website Docs in the repositories' README.md to mitigate questions like this.

Good idea. The documentation seems well written :)

From just my point of view, I would clarify the following terminology to make it easier for newbies to understand:

  • Options
  • Advanced options
  • Beautifier options
  • Configuration options

Perhaps only talk about "language options" and "beautifier options"? But this is minor.

Also, I had no idea clang-format could format Java and JavaScript, cool :)

@Glavin001
Copy link
Member

I recalled from some earlier issue that the plan was to always use Unibeautify's unified formatting options instead of beautifiers' native options, so I didn't think to look for a prefer_beautifier_config option.

You are absolutely correct 😃 . The objective is for all configuration to be unified by Unibeautify's options.

Unfortunately, we often have 2 problems:

  1. beautifier has options which are not applicable to the language in general
  2. configuring some beautifiers takes a lot more work than others, such as needing to create on-the-fly configuration file when there are no command-line arguments available. Both https://clang.llvm.org/docs/ClangFormat.html and https://github.com/FriendsOfPHP/PHP-CS-Fixer#usage fit into this group 👎 .

Our current workaround is to allow users to use existing configuration files. This allows us to support advanced configuration for specific beautifiers along with making the onboarding process easier for those already using these beautifiers and new to Unibeautify. For example, there may be tools in the future to convert from .clang-format configuration file to .unibeautifyrc.yml, however, it does not yet exist.


From just my point of view, I would clarify the following terminology to make it easier for newbies to understand:
...
Perhaps only talk about "language options" and "beautifier options"? But this is minor.

See https://unibeautify.com/docs/beautifier-clangformat.html#advanced :

image

You're right. These headers should be Beautifier Options and Language Options respectively, to align with the terminology used elsewhere.

@lassik Please feel free to make changes to https://github.com/Unibeautify/website . It is a work in progress and definitely could use clarify and improvements from newcomers. Thanks in advance! 😃

@lassik
Copy link
Contributor Author

lassik commented Jul 21, 2018

Awesome to hear 😄

Our current workaround is to allow users to use existing configuration files. This allows us to support advanced configuration for specific beautifiers along with making the onboarding process easier for those already using these beautifiers and new to Unibeautify.

Happy to hear about this. Sounds like the best approach to me too.

I'll see if I can find the time to go over the website. Thanks for soliciting feedback :)

@lassik
Copy link
Contributor Author

lassik commented Jul 21, 2018

configuring some beautifiers takes a lot more work than others, such as needing to create on-the-fly configuration file when there are no command-line arguments available. Both https://clang.llvm.org/docs/ClangFormat.html and https://github.com/FriendsOfPHP/PHP-CS-Fixer#usage fit into this group 👎 .

clang-format seems to be able to take the entire config on the command line via -style="{key: value, ...}". The format seems to be something very similar to JSON but not exactly (strings aren't quoted, or string quoting is optional). We are using this feature too, via generateConfigArgs() and loadConfigurationFromFile(): https://github.com/Unibeautify/beautifier-clang-format/blob/master/src/index.ts#L142 but the Unibeautify-to-clang-format translation of the options isn't implemented yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants