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.