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

automatic indentation (and style) #225

Open
bstarynk opened this issue Jun 9, 2018 · 4 comments
Open

automatic indentation (and style) #225

bstarynk opened this issue Jun 9, 2018 · 4 comments
Labels
help wanted This issue needs some community help to create a pull request.

Comments

@bstarynk
Copy link
Contributor

bstarynk commented Jun 9, 2018

I like a lot to use some indentation tool (e.g. GNU indent or astyle or any other free software indentation tool).

It would be very nice to publish your indentation style, perhaps as some .indent.pro file containing configuration for GNU indent (or some other configuration for some other indenting tool).

I don't care about what particular indentation style you use (honestly, I don't like it much -I prefer the usual GNU indentation style- and I don't exactly understand it, just am guessing it, but it is ok as long as you publish your style or give some indication on how to follow it, in particular how to reindent code for your tastes).

In an ideal world, I would be happy with some indent make target which would reindent automatically all the source code according to your tastes. This would facilitate external contributions.

@davidmoreno
Copy link
Owner

Please, submit a pull request with a proposal.

I did a quick check for indent and I liked the results of indent -linux -i2 -nut ...

The changes should add a target for the resulting makefiles to be able to do a make indent or similar.

@davidmoreno davidmoreno added the help wanted This issue needs some community help to create a pull request. label Jun 9, 2018
@bstarynk
Copy link
Contributor Author

bstarynk commented Jun 10, 2018

The main issue is for me and others to understand your indentation style. I cannot guess it (and in my eyes, it might be inconsistent and not well defined).

I don't understand what precise options to indent you have in mind (the comment above has ... and I cannot guess what should go inside it). If you decide that every C source file foo.c -for example src/onion/response.c- should be indented with (exactly) indent -linux -i2 -nut foo.c that is ok for me (but you need to make the decision and document it).

Once you have decided and agreed on some precise indentation, I suggest you to:

  • document your indentation style, perhaps in some CONTRIBUTING.md file (or just in the README.md); it could be as simple as "we follow the GNU formatting and indentation guidelines".

  • indent all the source files accordingly and git commit them (only you David can do that)

Once you defined an indentation style which can be automatized, implementing it in the Makefile is trivial. But an indentation style is a convention that only you can define.

If you want my personal opinion, I don't like your indentation in your C files, and I (Basile) prefer the GNU indentation style and to indent code with indent -gnu. But this is just my opinion (and habit), and I understand you have a different one.

For C++ code, you probably want to define indentation as arguments to astyle (since GNU indent don't really handle C++).

I believe that an indentation style can be arbitrary (it is a matter of taste) but should be documented and automatized (and if possible, not tied to one particular source code editor).

BTW, I will commit a patch for issue#224, but I probably will indent the entire modified files with indent -gnu (or with the exact indent command you want).

I tried to run indent -linux -i2 -nut parser.c on a copy of your onion/tools/otemplate/parser.c and the resulting file has a diff of 254 lines w.r.t. the original one.... So you need to decide and do something.

My personal wish is to adopt GNU indentation and to use indent -gnu on all your C files. But that makes a significant (but cosmetic) change to every file.

You can propose some other style, but you need to document it and provide some clues on how to automate it. The ability to automatize such formatting issues is more important than the particular style you choose to follow.

@davidmoreno
Copy link
Owner

Hi,

I added the contributing file (suggestions appreciated!) to master.

Also added make indent rule and indented code at the indent branch. Can you have a look and you feel its ok, I will merge it into master.

As a sidenote the "old" blame and diff can be seen adding the -w argument to both git blame -w and git diff -w so it ignores the big change which is the old inconsistent tabs to spaces.

@jeffvandyke
Copy link
Contributor

jeffvandyke commented Jul 23, 2018

Currently, I think that the file src/bindings/cpp/request.hpp looks particularly terrible. Mixed tabs and spaces, function names starting after the closing */ on the same line, and the opening doc comments /** start at column 17. Trailing whitespace is also an issue through the whole project. I think it could definitely be improved.

If you're looking for tools and options yet, probably the best tool to handle only whitespace for C/C++ would be ClangFormat, with rule options documented here. It basically works with a .clang-format file at the project root, specifying rules according to choice. Default .clang-format rules can be generated using the clang-format command for LLVM, Google, Chromium, Mozilla, or WebKit C++ style guides.

I can answer any questions, or work on a pull request if this is something the contributors or project owner is interested in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted This issue needs some community help to create a pull request.
Projects
None yet
Development

No branches or pull requests

3 participants