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

Code formatter #8

Merged
merged 6 commits into from
Jun 10, 2018
Merged

Code formatter #8

merged 6 commits into from
Jun 10, 2018

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Jun 5, 2018

This PR introduces Artistic Style code formatter and reformats the driver code with it.
The configuration of the formatter makes sure that no single-line blocks are left unbraced; this is valid for both conditionals and loops. A default of the formatter is also removal of trailing line spaces.

During the preparation I have already identified four locations (in two groups) where defects were introduced due to lacking braces; these have been committed part of this PR.

The formatting script will eventually be used in a pre-commit hook.

bpintea added 4 commits June 5, 2018 18:46
this type of bug will hopefully be no longer possible if always bracing
conditional-blocks
another bug due to missing braces
- add script to format the code with; the script checks against the
right (latest) version and calls the formatter with the arguments that
define the code style;
- add punctual pre-formatting changes, that would othewise lead to
  undesired results, when applying the formatter;
- disable formatting on some code chunks that the formatter doesn't
properly format:
  * do-while() interlaced in switch-cases;
  * one liner switch-cases (not supported by astyle).
code run through astyle
@bpintea bpintea requested review from droberts195 and edsavage June 5, 2018 21:11
@bpintea bpintea mentioned this pull request Jun 5, 2018
Closed
Copy link
Collaborator

@edsavage edsavage left a comment

Choose a reason for hiding this comment

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

LGTM

In spite of the number of sections where you've had to disable astyle formatting I think the over all result is well worth it.

)
{
esodbc_stmt_st *stmt = STMH(hstmt);
SQLRETURN ret;
SQLWCHAR wbuf[sizeof(SQL_COLUMNS(SQL_COL_CAT)) +
3 * ESODBC_MAX_IDENTIFIER_LEN];
3 * ESODBC_MAX_IDENTIFIER_LEN];
Copy link
Collaborator

Choose a reason for hiding this comment

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

The indentation is rather strange here - I'm not sure which of the astyle options is responsible though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. But changing the plus sign (b8cbdd9) makes things better.

switch(DiagIdentifier) {
/* Header Fields */
case SQL_DIAG_NUMBER:
if (StringLengthPtr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good that this was found!

bpintea added 2 commits June 6, 2018 15:51
async seems to treat differently the continuations of expressions,
depending on where the operators are placed.
- also fix a bug in formatter script parameters handling
@bpintea bpintea merged commit a67849a into elastic:master Jun 10, 2018
@bpintea bpintea deleted the feature/code_formatter branch June 10, 2018 09:48
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