Re: [Wireshark-dev] Editor config and code formatting
Hi, As consistent formatting not always translates into readable formatting (although in many/most cases it does) I’m not in favour of forced automatic formatting. And that is where great freedom comes with great responsibility. I’m already pleased if we can have a consistent style _per file_ I’ve created a mockup once, of a script calling uncrustify while adapting its configuration according to the modelines in the file. It works well, but readability sometimes suffers. My €0.02 > On 1 Mar 2022, at 18:45, David Perry wrote: > > Hi all, > > Bottom line up front: how much do people care about the formatting of > Wireshark's source code? > > Background: I'm looking into [#17253][1]. It's chiefly about removing editor > modelines from the footer of each source file in favour of just using > `.editorconfig` files. But by extension it's also about removing the > exceptions from `.editorconfig` files and making the formatting rules > consistent across files. > > I took a manual pass at harmonizing the formatting of the C files in the root > of the repo and that was painful, so I researched automatic approaches for > the rest of our code. [Clang-Format][2] seems to be a popular approach for > this sort of thing. > > Automatic code formatters in general, and clang-format in particular, are > rigid and somewhat naïve in how they do things. This is in contrast to the > flexible formatting practices we use. That's not a huge deal if we just want > to reformat once to harmonize our indentation levels and whatnot, and then > return to manually formatting based on the new standard. > > On the other hand, a comment on !6298 suggested that automatic reformatting > could be integrated as a pre-commit hook and/or a CI step. That... also isn't > a huge deal, I guess. We'd have consistency across files at the price of > slightly less formatting freedom. (And of having another developer > prerequisite to install, if we did it as a pre-commit hook.) > > But it's a decision that should be made by the dev community as a whole. So > what do you folks think? Is consistent formatting important to you? Would you > like to see it enforced with an automatic formatter? > > (My proposed `.clang-format` file is in [!6298][3] and aims to capture the > most common practices used across the codebase. Please use that MR for > discussions about specific formatting details. This email is for the general > discussion of whether/how to apply and enforce formatting.) > > Thanks for your time, > > David Perry > he/him > > [1]: https://gitlab.com/wireshark/wireshark/-/issues/17253 > [2]: https://releases.llvm.org/13.0.1/tools/clang/docs/ClangFormat.html > [3]: https://gitlab.com/wireshark/wireshark/-/merge_requests/6298 > ___ > Sent via:Wireshark-dev mailing list > Archives:https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev >mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Editor config and code formatting
EditorConfig has been in use with Wireshark for seven years at this point and it's still hit or miss to configure the indentation correctly for C. It's not a good tool to manage inconsistent styles. That's not what it was designed for. The only advantage EditorConfig has over modelines in that case is wider editor support. The fact that the setting is hidden away is actually a (minor) disadvantage if we still need to manage each individual file. On 02/03/22 12:36, João Valverde wrote: The policy that exists for new files is a mere recommendation to use 4 space indentation, and no one is enforcing it, nor really paying much attention to it. On 01/03/22 19:00, Roland Knall wrote: Policy always was and has been, that we try to achieve consistent guidelines for new files and in general the guidelines for each file should be reflecting that files style. Although I do appreciate applying consistent styles, I acknowledge the fact that we have a really old code base in some places and we shouldn’t force a change everywhere because of it. Can also see, that that would be neat with some opposition. So in general, although I appreciate having new files apply to style guides, I would keep the existing ones as is Cheers, Roland Am 01.03.2022 um 19:23 schrieb João Valverde : On 01/03/22 17:45, David Perry wrote: Hi all, Bottom line up front: how much do people care about the formatting of Wireshark's source code? I would like to have indentation harmonized (and enforced consistently) across the entire C code base. Preferably 4-space. Don't care so much about other style issues. I don't think that needs to be enforced. Background: I'm looking into [#17253][1]. It's chiefly about removing editor modelines from the footer of each source file in favour of just using `.editorconfig` files. But by extension it's also about removing the exceptions from `.editorconfig` files and making the formatting rules consistent across files. I took a manual pass at harmonizing the formatting of the C files in the root of the repo and that was painful, so I researched automatic approaches for the rest of our code. [Clang-Format][2] seems to be a popular approach for this sort of thing. Automatic code formatters in general, and clang-format in particular, are rigid and somewhat naïve in how they do things. This is in contrast to the flexible formatting practices we use. That's not a huge deal if we just want to reformat once to harmonize our indentation levels and whatnot, and then return to manually formatting based on the new standard. On the other hand, a comment on !6298 suggested that automatic reformatting could be integrated as a pre-commit hook and/or a CI step. That... also isn't a huge deal, I guess. We'd have consistency across files at the price of slightly less formatting freedom. (And of having another developer prerequisite to install, if we did it as a pre-commit hook.) But it's a decision that should be made by the dev community as a whole. So what do you folks think? Is consistent formatting important to you? Would you like to see it enforced with an automatic formatter? (My proposed `.clang-format` file is in [!6298][3] and aims to capture the most common practices used across the codebase. Please use that MR for discussions about specific formatting details. This email is for the general discussion of whether/how to apply and enforce formatting.) Thanks for your time, David Perry he/him [1]: https://gitlab.com/wireshark/wireshark/-/issues/17253 [2]: https://releases.llvm.org/13.0.1/tools/clang/docs/ClangFormat.html [3]: https://gitlab.com/wireshark/wireshark/-/merge_requests/6298 ___ Sent via: Wireshark-dev mailing list Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via: Wireshark-dev mailing list Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via: Wireshark-dev mailing list Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via: Wireshark-dev mailing list Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Editor config and code formatting
Always difficult to comment on such things as I'm a hard-line centrist, but I would be in favour of enforcing both indents and style\formatting. If we had enforced style, we wouldn't be having this discussion, nor would we need to even think about it when writing code, it would just happen automagically. An enforced style (that would hopefully match a majority of existing code) will obviously tweak some folks' brace alignment views but I'm sure we could then all move on to more important things. On Wed, 2 Mar 2022 at 12:37, João Valverde wrote: > The policy that exists for new files is a mere recommendation to use 4 > space indentation, and no one is enforcing it, nor really paying much > attention to it. > > On 01/03/22 19:00, Roland Knall wrote: > > Policy always was and has been, that we try to achieve consistent > guidelines for new files and in general the guidelines for each file should > be reflecting that files style. > > > > Although I do appreciate applying consistent styles, I acknowledge the > fact that we have a really old code base in some places and we shouldn’t > force a change everywhere because of it. > > > > Can also see, that that would be neat with some opposition. So in > general, although I appreciate having new files apply to style guides, I > would keep the existing ones as is > > > > Cheers, > > Roland > > > >> Am 01.03.2022 um 19:23 schrieb João Valverde : > >> > >> On 01/03/22 17:45, David Perry wrote: > >>> Hi all, > >>> > >>> Bottom line up front: how much do people care about the formatting of > Wireshark's source code? > >> I would like to have indentation harmonized (and enforced consistently) > across the entire C code base. Preferably 4-space. > >> > >> Don't care so much about other style issues. I don't think that needs > to be enforced. > >> > >>> Background: I'm looking into [#17253][1]. It's chiefly about removing > editor modelines from the footer of each source file in favour of just > using `.editorconfig` files. But by extension it's also about removing the > exceptions from `.editorconfig` files and making the formatting rules > consistent across files. > >>> > >>> I took a manual pass at harmonizing the formatting of the C files in > the root of the repo and that was painful, so I researched automatic > approaches for the rest of our code. [Clang-Format][2] seems to be a > popular approach for this sort of thing. > >>> > >>> Automatic code formatters in general, and clang-format in particular, > are rigid and somewhat naïve in how they do things. This is in contrast to > the flexible formatting practices we use. That's not a huge deal if we just > want to reformat once to harmonize our indentation levels and whatnot, and > then return to manually formatting based on the new standard. > >>> > >>> On the other hand, a comment on !6298 suggested that automatic > reformatting could be integrated as a pre-commit hook and/or a CI step. > That... also isn't a huge deal, I guess. We'd have consistency across files > at the price of slightly less formatting freedom. (And of having another > developer prerequisite to install, if we did it as a pre-commit hook.) > >>> > >>> But it's a decision that should be made by the dev community as a > whole. So what do you folks think? Is consistent formatting important to > you? Would you like to see it enforced with an automatic formatter? > >>> > >>> (My proposed `.clang-format` file is in [!6298][3] and aims to capture > the most common practices used across the codebase. Please use that MR for > discussions about specific formatting details. This email is for the > general discussion of whether/how to apply and enforce formatting.) > >>> > >>> Thanks for your time, > >>> > >>> David Perry > >>> he/him > >>> > >>> [1]: https://gitlab.com/wireshark/wireshark/-/issues/17253 > >>> [2]: > https://releases.llvm.org/13.0.1/tools/clang/docs/ClangFormat.html > >>> [3]: https://gitlab.com/wireshark/wireshark/-/merge_requests/6298 > >>> > ___ > >>> Sent via:Wireshark-dev mailing list > >>> Archives:https://www.wireshark.org/lists/wireshark-dev > >>> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > >>> mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe > >> > ___ > >> Sent via:Wireshark-dev mailing list > >> Archives:https://www.wireshark.org/lists/wireshark-dev > >> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > >> mailto:wireshark-dev-requ...@wireshark.org > ?subject=unsubscribe > > > ___ > > Sent via:Wireshark-dev mailing list > > Archives:https://www.wireshark.org/lists/wireshark-dev > > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > > mailto:wireshark-dev-requ...@wireshark.org >
Re: [Wireshark-dev] Editor config and code formatting
The policy that exists for new files is a mere recommendation to use 4 space indentation, and no one is enforcing it, nor really paying much attention to it. On 01/03/22 19:00, Roland Knall wrote: Policy always was and has been, that we try to achieve consistent guidelines for new files and in general the guidelines for each file should be reflecting that files style. Although I do appreciate applying consistent styles, I acknowledge the fact that we have a really old code base in some places and we shouldn’t force a change everywhere because of it. Can also see, that that would be neat with some opposition. So in general, although I appreciate having new files apply to style guides, I would keep the existing ones as is Cheers, Roland Am 01.03.2022 um 19:23 schrieb João Valverde : On 01/03/22 17:45, David Perry wrote: Hi all, Bottom line up front: how much do people care about the formatting of Wireshark's source code? I would like to have indentation harmonized (and enforced consistently) across the entire C code base. Preferably 4-space. Don't care so much about other style issues. I don't think that needs to be enforced. Background: I'm looking into [#17253][1]. It's chiefly about removing editor modelines from the footer of each source file in favour of just using `.editorconfig` files. But by extension it's also about removing the exceptions from `.editorconfig` files and making the formatting rules consistent across files. I took a manual pass at harmonizing the formatting of the C files in the root of the repo and that was painful, so I researched automatic approaches for the rest of our code. [Clang-Format][2] seems to be a popular approach for this sort of thing. Automatic code formatters in general, and clang-format in particular, are rigid and somewhat naïve in how they do things. This is in contrast to the flexible formatting practices we use. That's not a huge deal if we just want to reformat once to harmonize our indentation levels and whatnot, and then return to manually formatting based on the new standard. On the other hand, a comment on !6298 suggested that automatic reformatting could be integrated as a pre-commit hook and/or a CI step. That... also isn't a huge deal, I guess. We'd have consistency across files at the price of slightly less formatting freedom. (And of having another developer prerequisite to install, if we did it as a pre-commit hook.) But it's a decision that should be made by the dev community as a whole. So what do you folks think? Is consistent formatting important to you? Would you like to see it enforced with an automatic formatter? (My proposed `.clang-format` file is in [!6298][3] and aims to capture the most common practices used across the codebase. Please use that MR for discussions about specific formatting details. This email is for the general discussion of whether/how to apply and enforce formatting.) Thanks for your time, David Perry he/him [1]: https://gitlab.com/wireshark/wireshark/-/issues/17253 [2]: https://releases.llvm.org/13.0.1/tools/clang/docs/ClangFormat.html [3]: https://gitlab.com/wireshark/wireshark/-/merge_requests/6298 ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
