Re: Results of: [VOTE] New space-before-parens style

kfogel@collab.net wrote:
> Okay, I think it's ready to run. Here's a diff first, so y'all can
> check my work: http://www.red-bean.com/kfogel/reformat.diff.
>
> Yes, Subversion still compiles afterwards. But I won't commit until I
> hear that Julian Foad has looked at the diff :-).

:-)

OK, the first thing is it doesn't handle open-paren-at-eol properly:

> svn_error_t *
> -svn_cmdline_auth_ssl_client_cert_prompt (
> - svn_auth_cred_ssl_client_cert_t **cred_p,
> - void *baton,
> - const char *realm,
> - svn_boolean_t may_save,
> - apr_pool_t *pool);
> +svn_cmdline_auth_ssl_client_cert_prompt(
> + svn_auth_cred_ssl_client_cert_t **cred_p,
> + void *baton,
> + const char *realm,
> + svn_boolean_t may_save,
> + apr_pool_t *pool);

There are about 50 of these, probably just few enough that you could fix them
up by hand; and anyway I think our preferred style here is to move the paren to
the next line, so you might as well do that while you're there.

Here's another problem (one instance happens to have a paren at EOL):

> - typedef BOOL (CALLBACK *encrypt_fn_t)(
> + typedef BOOL(CALLBACK *encrypt_fn_t)(
> DATA_BLOB *, /* pDataIn */

> -typedef svn_boolean_t (*password_set_t) (apr_hash_t *creds,
> +typedef svn_boolean_t(*password_set_t) (apr_hash_t *creds,

> typedef apr_pool_t *(*svn_swig_pl_get_current_pool_t) (void);
> typedef void (*svn_swig_pl_set_current_pool_t) (apr_pool_t *pool);

For these I searched for "typedef.*(.*(". Spaces after the return type are
removed, and spaces before argument lists are missed.

Similarly, but without "typedef", parenthesised functions are missed:

> err = (*subcommand->cmd_func) (os, &opt_state, pool);

> (*fs->warning) (fs->warning_baton, err);

> +do_retry(svn_fs_t *fs,
> + svn_error_t *(*txn_body) (void *baton, trail_t *trail),

For this I searched for ") *(". Personally I'd be happy to have no space after
type casts too:

> diffdata[3] = (char) (first_chunk->version);

That wasn't explicitly part of what we voted on, but we're presently
inconsistent and often have no space.

It misses where there is more than one space:

> - SVN_ERR (svn_fs_make_file (txn_root, "q", pool));
> - SVN_ERR (svn_fs_make_dir (txn_root, "A", pool));
> + SVN_ERR(svn_fs_make_file(txn_root, "q", pool));
> + SVN_ERR(svn_fs_make_dir (txn_root, "A", pool));

There are not many cases of that, I think. I don't think we should worry too
much about keeping the horizontal alignment of cases like the one I quoted
here, but if you edit this by hand I'd say it's OK to keep a space after "_dir"
there to keep the alignment. (This quote is from a longer section so the
alignment is worth something.)

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Feb 15 18:33:59 2006