Add two beautifiers: clang-format and yapf #546
Conversation
| options: { | ||
| "C++": true | ||
| "C": true | ||
| "Objective-C": true |
There was a problem hiding this comment.
It does not look like any of the options are used, so change it from true to false to indicate that while the language is supported it does not support any of the options directly.
This is important for the automatic documentation generation.
There was a problem hiding this comment.
Thanks for pointing it out. Since clang-format uses its own configuration file, it does not make too much sense to accept options. And projects using clang-format usually provides their own configuration file. I will change it in a new commit.
| ], help: { | ||
| link: "https://clang.llvm.org/docs/ClangFormat.html" | ||
| }).then( => | ||
| fs.unlinkSync(dumpFile) |
There was a problem hiding this comment.
Could you make this not the sync function call? Wouldn't want to lock up Atom is the hard drive is slow. This will allow other things to be processed if the hard drive is taking time.
There was a problem hiding this comment.
Thanks, Glavin. I have made necessary changes and they are waiting for your review. However, a strange issue arises in the process. The beautifier does nothing to the buffer with atom-1.0.15, while it works correctly under atom-1.0.9. It can be fixed by changing the last ".then" to ".finally". I am new to the javascript world and do not quite understand the Promise mechanisms. Would you please suggest a better or more idiomatic approach to delete the temp file after the @run call?
There was a problem hiding this comment.
then will be called if all of the Promises passed, successfully. finally will be called regardless of if it was successful (then) or an error (catch). So it sounds like an error was occuring and it was not running the then callback. Makes good sense to put it under the finally callback.
|
I am fixing the tests now. Please re-merge the latest changes, fix the above comments, and I can review your Pull request and merge! 😃 Thank you! |
| ) | ||
| .then((dumpFile) => | ||
| # console.log("clang-format", dumpFile) | ||
| @run("clang-format", [ |
There was a problem hiding this comment.
I think this needs to be returned.
return @run(...
therefore its results (success or error) are passed up to the promise above, which is already returned.
There was a problem hiding this comment.
Actually maybe CoffeeScript does correctly return this one already.
I was just thinking that one of your comments sounded like an error was occurring (then not being called) and I thought maybe this could be why.
Were you able to have a test that did return an error and properly notify you?
There was a problem hiding this comment.
I have tried the following tests:
A. Null workspace. By running beautify buffer on the settings tab, an error is notified by atom-beautify, with the content no workspace found.
B. Readonly directory. By changing the mode of the containing directory to 'u-w', an error with EACCS is notified. This works with and without return @run....
I doubt that chrome changes its implementation of promises, since atom upgrades its chrome to 43? in at least 1.0.15. The then works correctly (the temp file is deleted) but the stdout of @run is not passed out correctly. Although I am still curious why it works, the finally solves the problem.
|
I think this is almost ready to merge! Looking at the failing tests, it appears that linting has failed. You can run |
|
The lint errors are fixed. It seems that the appveyor build failed because the script return 1 instead of 0. The build logs shows 0 errors. So don't know why it failed. |
Add two beautifiers: clang-format and yapf
|
Thank you! |
|
It's a pleasure to contribute. :) |
|
Published to v0.28.15 |
clang-format is a semantics-aware C/C++/Objective-C beautifier from clang compiler tool. It produces correct code even for very complicated sources. So this can be an alternative to uncrusify for C/C++/Obj-C.
yapf (Yet Another Python Formatter) is a python formatter using the same format algorithm as clang-format. It's said to produce better looking pep8-conforming code than autopep8. So someone might want it.
This pull request implements two beautifiers for the above code formatters. I have tested it with my own code base and it works fine.