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

Add Kernighan & Ritchie style brace formatting #534

Merged
merged 3 commits into from
Oct 9, 2021
Merged

Conversation

s-ludwig
Copy link
Contributor

The original K&R style behaves like BraceStyle.otbs, except for function definitions that use Allman style.

See https://en.wikipedia.org/wiki/Indentation_style#K&R_style

The original K&R style behaves like BraceStyle.otbs, except for function definitions that use Allman style.
@s-ludwig
Copy link
Contributor Author

Should be noted that I've taken the "otbs" tests and adjusted them by hand to fit the "knr" style.

@s-ludwig
Copy link
Contributor Author

Any chance of someone having a look at this? Would be nice to be able to use dfmt as a tool for contributions to vibe.d related projects.

Note that the actual change is pretty small, it's just a lot of test cases added.

@rikkimax rikkimax requested a review from Hackerpilot August 16, 2021 14:44
{
if (auto bd = functionBody.specifiedFunctionBody)
{
astInformation.funBodyLocations ~= bd.blockStatement.startLocation;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there could be a null dereference here if bd.blockStatement is null.

However with the current grammar this case is impossible, it could only be introduced in the future. I don't think it's very likely that this will happen to blockStatement (as shortened statement got a separate AST name), so I am fine with merging without checking for null here

@WebFreak001
Copy link
Member

I think in general it would be better if all of these states were tweakable separately, but would merge this one for now anyway. Would like to know what @Hackerpilot thinks before merging too though.

@s-ludwig
Copy link
Contributor Author

Agreed that separate switches would be preferable (IMO), but I gathered that the project vision was to be more opinionated rather than supporting customization. @Hackerpilot doesn't seem to be engaged much with open-source projects lately, though, so the question is how/if we can continue with this.

@WebFreak001
Copy link
Member

would merge today unless he appears then

@s-ludwig
Copy link
Contributor Author

s-ludwig commented Oct 8, 2021

Still under the radar...

@WebFreak001
Copy link
Member

thanks for reminder

@WebFreak001 WebFreak001 merged commit f1e5713 into master Oct 9, 2021
@WebFreak001 WebFreak001 deleted the knr_brace_style branch October 9, 2021 07:09
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.

2 participants