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

4 spaces indentation #122

Conversation

Janther
Copy link
Contributor

@Janther Janther commented Mar 20, 2019

Solidity industry indents using 4 spaces and this opinionated code formatter should follow the industry.

@codecov
Copy link

codecov bot commented Mar 20, 2019

Codecov Report

Merging #122 into master will decrease coverage by 0.76%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #122      +/-   ##
=========================================
- Coverage   95.27%   94.5%   -0.77%     
=========================================
  Files           5       5              
  Lines         254     255       +1     
  Branches       56      56              
=========================================
- Hits          242     241       -1     
- Misses         12      14       +2
Impacted Files Coverage Δ
src/index.js 94.11% <100%> (+0.36%) ⬆️
src/clean.js 50% <0%> (-50%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f80a9a8...90474d2. Read the comment docs.

@mattiaerre
Copy link
Member

I've got no particular preference but I see there are some strange indentations in the snapshots?

@Janther
Copy link
Contributor Author

Janther commented Mar 20, 2019

everything that is code related has 4 spaces indentation (I changed these manually before running the tests).
However, block comments look a bit weird. I added a TODO line

Also, snapshots contain the code before and after the format so you might also be looking at the code before the format, which contains different types of indentation (2 spaces, 4 spaces, tabs, etc) and trailing whitespace.

@fvictorio
Copy link
Member

LGTM. @Janther, can you give an example of some block comment that looks weird after being formatted?

@Janther
Copy link
Contributor Author

Janther commented Mar 24, 2019

For example, this is the block before formatting:

/**********
Standard kill() function to recover funds
**********/

and after formatting it looks like this:

/**********
Standard kill() function to recover funds
**********/

also tabs are kept within these blocks

/*
This is a very simple demonstration of a while loops. Same as JS/c.
*/

@Janther
Copy link
Contributor Author

Janther commented Mar 24, 2019

trying these out with prettier on javascript, it has the same issue.
I think the Prettier team doesn't mind.

@fvictorio
Copy link
Member

Yeah, the formatting of comments is handled by prettier itself. If the same thing happens in js, I think we can ignore it here.

Keeping the tabs seems OK to me: it makes sense to keep the comment's content verbatim.

The first example does look wrong though. Maybe you can open an issue in Prettier's repo?

@mattiaerre
Copy link
Member

if we are all happy I'll merge and deploy

@mattiaerre mattiaerre merged commit b994774 into prettier-solidity:master Mar 28, 2019
@Janther Janther deleted the feature/4-space-indentation branch June 5, 2019 23:48
tash-2s added a commit to tash-2s/onchain-game-2019_archive that referenced this pull request Oct 5, 2023
I need to clearly specify `tabWidth: 2` because of this:
4 spaces indentation by Janther · Pull Request #122 · prettier-solidity/prettier-plugin-solidity prettier-solidity/prettier-plugin-solidity#122

---

Original commit (chain-contracts): 87b07af95d503cdd51a64b1c6a6ee078b7fd9525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants