[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 05 Apr 2015 12:43:23 +0200
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: Coding Style
On 2015-04-05 12:02, Solar Designer wrote:
> On Sun, Apr 05, 2015 at 05:44:22AM +0200, magnum wrote:
>> Unfortunately a lot of the code deviate from our coding style.
>
> Yes. Given how differently-formatted code is constantly being
> contributed to jumbo, maybe we should adopt a policy to reformat all of
> jumbo with particular indent(1) settings?
Maybe. I have contemplated creating local commit hooks that we can
provide to developers, so they simply can't commit bad stuff (or in some
cases if we want to, they can but it will silently be fixed). GitHub
doesn't support commit hooks as they present a heavy security challenge
so everyone would need to install them locally. This would also finally
put an end to Jim's habit of committing CRLF lines in files that are
otherwise LF only (I can't fathom how any editor can do such a thing).
> As discussed before, this is about the closest match to what we need:
>
> indent -kr -i8 -nlp -nbbo -ncs -l79 -lc79
>
> although I dislike a few things about it - e.g., how it indents goto
> labels. Oh, and IIRC our fmt_main struct initializers are mangled by
> this really bad.
>
> Maybe we should patch indent(1) for our use?
I think GNU indent has more options that are applicabe to us, than the
most basic variants. I *think* I could get around the struct issue but I
don't have any notes left from that. Someone should investigate what can
be used (and we should list it as an alternative for GNU indent only)
> Another drawback of this mass-reformat is that the core tree of JtR
> intentionally uses custom formatting in a few places. I guess I'd have
> to give up on this, just to avoid unnecessary diffs with jumbo.
Speaking of core. Jumbo should strive to not change core, only add to it
(when possible, of course). For this reason my patches to core files
often has non-standard formatting too, eg.
crk_key_index = 0;
crk_last_salt = NULL;
+ if (options.flags & FLG_MASK_STACKED)
+ mask_fix_state();
+ else
crk_fix_state();
Here, the crk_fix_state was not indented - and this was intentional. The
more we do it like that, the less merge conflicts I get when merging core.
BTW I am tempted to officially have Jumbo deviate from core's tab width
recommentdation, using 4 instead of 8. With the "indent with TABs, align
with spaces" requirement (which I'm prepared to kill for) this only
affects how long a line gets[*]. It seems to me absolutely no-one use 8
(except I use to do it when working with John - but I'm about to give
that up) so many lines are committed that did not exceed column 80 with
the author's TAB setting but do when 8 is used.
[*] well it also affects things like "#define foo <tab> <tab> bar" which
we ideally should use space for to make things even more tab-width
agnostic. But I have yet to find out how to learn emacs to do that for me.
magnum
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
