Re: [PATCH] src/*.cpp: reformatting to increase consistency
On Sun, Oct 28, 2012 at 01:46:05PM +0100, Stephan Witt wrote: > >> Can you point me to the rules for coding style please? > > > > I only know of the old files in development/coding. > > The best match I found is: > > - Adapt the formatting of your code to the one used in the >other parts of LyX. In case there is different formatting for >the same construct, use the one used more often. > > So this implies one should care for *own* code, IMHO. The vague formulation of "In case there is different formatting for the same construct, use the one used more often." was a result of LyX not having a lot of written, but quite a few unwritten rules, and the "majority vote" pretty much gives the same result as having all rules explicit. It was meant as a general permission, or even encouragement, to strive for a uniform code base, especially, but not restricted to cases where there are witten rules that are followed, and if the reformatting (as was e.g. done by Lars here) is separated from functional changes. It is really easier to apply mechanical refactoring on top of a uniform code base and to spot deviation from existing known -to-work-well patterns easier this way. Andre'
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Stephan Witt writes: | Am 27.10.2012 um 22:21 schrieb Lars Gullik Bjønnes : > >> On 27 October 2012 21:31, Stephan Witt wrote: >>> Am 27.10.2012 um 21:12 schrieb Lars Gullik Bjønnes : >>> > This invalidates all patches hanging around for cosmetic reasons. > Is this really necessary? Why do you have patches hanging around? >>> >>> Because I didn't apply them? >> >> Commit them to one or more branches and I'll rebase them to HEAD for you. > | Thanks, but Abdel is probably right. I can easily do this myself. > >> Are you saying that changes are hostage to undisclosed patches that someone might have? >>> >>> No, I asked if it's necessary to change the code for increased >>> consistency of coding style. >> >> Not necessary, but nice. Esp. consistency within a single file. >> >>> Can you point me to the rules for coding style please? >> >> I only know of the old files in development/coding. > | The best match I found is: > | - Adapt the formatting of your code to the one used in the |other parts of LyX. In case there is different formatting for |the same construct, use the one used more often. > | So this implies one should care for *own* code, IMHO. What if "one" has not done this? Who will ever to it then? (I'd call it a "bug" in the commit even, even if it can feel overly pedantic I do belive that should be part of review comments.) | I can understand your goal and I'm often tempted to change the formatting of our code base at work myself. | But because it is a matter of taste until it's not well defined there is some chance of "code format bouncing". It is pretty well defined how we format code in LyX. We can get some bouncing of course, but sometime it is not obvious how to format everyting. esp. when it compes to overlong constructs and who to make it more readable. (As as to work... there I do it completely different. You wouldn't want that style here.) -- Lgb
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Am 27.10.2012 um 22:21 schrieb Lars Gullik Bjønnes : > On 27 October 2012 21:31, Stephan Witt wrote: >> Am 27.10.2012 um 21:12 schrieb Lars Gullik Bjønnes : >> This invalidates all patches hanging around for cosmetic reasons. Is this really necessary? >>> >>> Why do you have patches hanging around? >> >> Because I didn't apply them? > > Commit them to one or more branches and I'll rebase them to HEAD for you. Thanks, but Abdel is probably right. I can easily do this myself. > >>> Are you saying that changes are hostage to undisclosed patches that >>> someone might have? >> >> No, I asked if it's necessary to change the code for increased consistency >> of coding style. > > Not necessary, but nice. Esp. consistency within a single file. > >> Can you point me to the rules for coding style please? > > I only know of the old files in development/coding. The best match I found is: - Adapt the formatting of your code to the one used in the other parts of LyX. In case there is different formatting for the same construct, use the one used more often. So this implies one should care for *own* code, IMHO. I can understand your goal and I'm often tempted to change the formatting of our code base at work myself. But because it is a matter of taste until it's not well defined there is some chance of "code format bouncing". This said, I'll don't oppose to this patch. Stephan
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Hi Stephan, On 27/10/2012 20:56, Stephan Witt wrote: This invalidates all patches hanging around for cosmetic reasons. Is this really necessary? Stephan Coding style and consistency is always worth it IMO. Solving merge issues because of those are often easy. But this was a very valid question :-) Cheers, Abdel.
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Hi Lars, I didn't read in detail but I think this patch is good. Thanks, Abdel
Re: [PATCH] src/*.cpp: reformatting to increase consistency
On 27 October 2012 21:31, Stephan Witt wrote: > Am 27.10.2012 um 21:12 schrieb Lars Gullik Bjønnes : > >>> This invalidates all patches hanging around for cosmetic reasons. >>> Is this really necessary? >> >> Why do you have patches hanging around? > > Because I didn't apply them? Commit them to one or more branches and I'll rebase them to HEAD for you. >> Are you saying that changes are hostage to undisclosed patches that >> someone might have? > > No, I asked if it's necessary to change the code for increased consistency of > coding style. Not necessary, but nice. Esp. consistency within a single file. > Can you point me to the rules for coding style please? I only know of the old files in development/coding. -- Lgb
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Am 27.10.2012 um 21:12 schrieb Lars Gullik Bjønnes : >> This invalidates all patches hanging around for cosmetic reasons. >> Is this really necessary? > > Why do you have patches hanging around? Because I didn't apply them? > Are you saying that changes are hostage to undisclosed patches that > someone might have? No, I asked if it's necessary to change the code for increased consistency of coding style. Can you point me to the rules for coding style please? > And if it takes you more than 5 minutes to fix the conflicts that you > get, of all > will be trivial. Then I'll go crawl back under a rock again. My opinion is not that important… Stephan
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Stephan Witt writes: | Am 27.10.2012 um 15:46 schrieb Lars Gullik Bjønnes : > >> --- >> src/AppleSpellChecker.cpp | 6 +- >> src/AspellChecker.cpp | 32 +++-- >> src/Author.cpp| 13 +- >> src/BiblioInfo.cpp| 233 +- >> src/Bidi.cpp | 1 + >> src/BranchList.cpp| 5 +- >> src/Buffer.cpp| 16 ++- >> src/BufferList.cpp| 2 + >> src/BufferParams.cpp | 5 +- >> src/BufferView.cpp| 1 + >> src/Changes.cpp | 2 + >> src/Chktex.cpp| 3 +- >> src/CmdDef.cpp| 2 +- >> src/Color.cpp | 1 + >> src/Compare.cpp | 28 +++-- >> src/ConverterCache.cpp| 6 +- >> src/CoordCache.cpp| 1 + >> src/Counters.cpp | 2 +- >> src/Cursor.cpp| 314 >> +++--- >> src/CutAndPaste.cpp | 1 + >> src/DepTable.cpp | 6 +- >> src/DocIterator.cpp | 1 + >> src/EnchantChecker.cpp| 12 +- >> src/Encoding.cpp | 3 +- >> src/FontInfo.cpp | 192 ++-- >> src/Format.cpp| 15 ++- >> src/FuncStatus.cpp| 3 +- >> src/HSpace.cpp| 1 + >> src/HunspellChecker.cpp | 34 +++-- >> src/IndicesList.cpp | 8 +- >> src/LaTeXFeatures.cpp | 2 + >> src/Layout.cpp| 19 +-- >> src/LayoutFile.cpp| 11 +- >> src/LyX.cpp | 10 +- >> src/ModuleList.cpp| 10 +- >> src/Paragraph.cpp | 32 +++-- >> src/ParagraphMetrics.cpp | 3 +- >> src/PersonalWordList.cpp | 1 + >> src/Server.cpp| 8 +- >> src/ServerSocket.cpp | 1 - >> src/Session.cpp | 1 - >> src/Text.cpp | 1 + >> src/TextClass.cpp | 92 +++--- >> src/Thesaurus.cpp | 9 +- >> src/VCBackend.cpp | 1 + >> src/VSpace.cpp| 1 + >> src/WordList.cpp | 3 +- >> src/factory.cpp | 15 ++- >> src/lengthcommon.cpp | 2 + >> src/lyxfind.cpp | 25 ++-- >> src/output_docbook.cpp| 49 >> src/output_latex.cpp | 6 +- >> src/output_plaintext.cpp | 1 + >> src/output_xhtml.cpp | 17 +-- >> src/rowpainter.cpp| 2 + >> 55 files changed, 719 insertions(+), 552 deletions(-) > | This invalidates all patches hanging around for cosmetic reasons. | Is this really necessary? I can spread it out if you want to. Only fix inconsistencies when I am going to make some other changes anyway. But I do not understand why some patches that "hang around" should be allowed to decide anything. -- Lgb
Re: [PATCH] src/*.cpp: reformatting to increase consistency
> This invalidates all patches hanging around for cosmetic reasons. > Is this really necessary? Why do you have patches hanging around? Are you saying that changes are hostage to undisclosed patches that someone might have? And if it takes you more than 5 minutes to fix the conflicts that you get, of all will be trivial. Then I'll go crawl back under a rock again. -- Lgb
Re: [PATCH] src/*.cpp: reformatting to increase consistency
Am 27.10.2012 um 15:46 schrieb Lars Gullik Bjønnes : > --- > src/AppleSpellChecker.cpp | 6 +- > src/AspellChecker.cpp | 32 +++-- > src/Author.cpp| 13 +- > src/BiblioInfo.cpp| 233 +- > src/Bidi.cpp | 1 + > src/BranchList.cpp| 5 +- > src/Buffer.cpp| 16 ++- > src/BufferList.cpp| 2 + > src/BufferParams.cpp | 5 +- > src/BufferView.cpp| 1 + > src/Changes.cpp | 2 + > src/Chktex.cpp| 3 +- > src/CmdDef.cpp| 2 +- > src/Color.cpp | 1 + > src/Compare.cpp | 28 +++-- > src/ConverterCache.cpp| 6 +- > src/CoordCache.cpp| 1 + > src/Counters.cpp | 2 +- > src/Cursor.cpp| 314 +++--- > src/CutAndPaste.cpp | 1 + > src/DepTable.cpp | 6 +- > src/DocIterator.cpp | 1 + > src/EnchantChecker.cpp| 12 +- > src/Encoding.cpp | 3 +- > src/FontInfo.cpp | 192 ++-- > src/Format.cpp| 15 ++- > src/FuncStatus.cpp| 3 +- > src/HSpace.cpp| 1 + > src/HunspellChecker.cpp | 34 +++-- > src/IndicesList.cpp | 8 +- > src/LaTeXFeatures.cpp | 2 + > src/Layout.cpp| 19 +-- > src/LayoutFile.cpp| 11 +- > src/LyX.cpp | 10 +- > src/ModuleList.cpp| 10 +- > src/Paragraph.cpp | 32 +++-- > src/ParagraphMetrics.cpp | 3 +- > src/PersonalWordList.cpp | 1 + > src/Server.cpp| 8 +- > src/ServerSocket.cpp | 1 - > src/Session.cpp | 1 - > src/Text.cpp | 1 + > src/TextClass.cpp | 92 +++--- > src/Thesaurus.cpp | 9 +- > src/VCBackend.cpp | 1 + > src/VSpace.cpp| 1 + > src/WordList.cpp | 3 +- > src/factory.cpp | 15 ++- > src/lengthcommon.cpp | 2 + > src/lyxfind.cpp | 25 ++-- > src/output_docbook.cpp| 49 > src/output_latex.cpp | 6 +- > src/output_plaintext.cpp | 1 + > src/output_xhtml.cpp | 17 +-- > src/rowpainter.cpp| 2 + > 55 files changed, 719 insertions(+), 552 deletions(-) This invalidates all patches hanging around for cosmetic reasons. Is this really necessary? Stephan
