Hi Nick, Enrico,
I'm somewhat embarrassed by the number of style fixes in build.c when I thought it was better than that
How much of the changes are manual and how much is done by the fix_alignment script & is it safe to just run this every time pre-commit?
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code". Then I could have saved you some work :-) At least the things not addressed by the script should be listed, eg when and if {} is used around single if clauses etc.
Cheers Lex
Lex Trotman schrieb:
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code".
This is what I had an argument with Enrico too recently. HACKING is nowhere near accurate, as it's poorly describing what style Enrico is expecting (Nick apparently not so, since he accepted my patch). In addition, seem Enrico and Nick run with extra CFLAGS which reveal extra warnings (some of which warn for perfectly conform C code, but well ;) ) which are not documented.
"like the rest of the code" is a very poor and inaccurate statement. Besides, that this is basically a http://dict.leo.org/ende?lp=ende&p=12RDU.&search=a matter http://dict.leo.org/ende?lp=ende&p=12RDU.&search=matter of http://dict.leo.org/ende?lp=ende&p=12RDU.&search=of course http://dict.leo.org/ende?lp=ende&p=12RDU.&search=course
Enrico said he'd update HACKING, but that didn't happen yet if I see this correctly (haven't checked SVN log since a day or too).
Then I could have saved you some work :-) At least the things not addressed by the script should be listed, eg when and if {} is used around single if clauses etc.
Cheers Lex _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Thomas Martitz schrieb:
Lex Trotman schrieb:
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code".
This is what I had an argument with Enrico too recently. HACKING is nowhere near accurate, as it's poorly describing what style Enrico is expecting (Nick apparently not so, since he accepted my patch). In addition, seem Enrico and Nick run with extra CFLAGS which reveal extra warnings (some of which warn for perfectly conform C code, but well ;) ) which are not documented.
"like the rest of the code" is a very poor and inaccurate statement. Besides, that this is basically a http://dict.leo.org/ende?lp=ende&p=12RDU.&search=a matter http://dict.leo.org/ende?lp=ende&p=12RDU.&search=matter of http://dict.leo.org/ende?lp=ende&p=12RDU.&search=of course http://dict.leo.org/ende?lp=ende&p=12RDU.&search=course
ARG, ctrl+enter sucks :p
So, to finish of my mail: "like the rest of the code" is a poor and inaccurate statement. Besides, that this is basically true for any project I've seen, it doesn't help you identifying tiny stuff such as trying to avoid shadow declarations or indentation when breaking long lines and stuff. HACKING should be as explicit as possible, to avoid additional work for *both*, the contributors and the reviewers.
Best regards.
On Mon, 07 Sep 2009 00:43:02 +0200, Thomas wrote:
Thomas Martitz schrieb:
Lex Trotman schrieb:
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code".
This is what I had an argument with Enrico too recently. HACKING is nowhere near accurate, as it's poorly describing what style Enrico
As already said, I try to update the HACKING file soon. Still, in the argument we had recently, I told you that some of the issues I had with your coding style we had already discussed in the past (e.g. in the snippets patches). But you are right, if the style guidelines would be better and more detailed described, we could have saved this conversion (and maybe even this one). It's somewhat my bad, no doubt.
In addition, seem Enrico and Nick run with extra CFLAGS which reveal extra warnings (some of which warn for perfectly conform C code, but well ;) ) which are not documented.
They are not "documented" because these flags are not required not suggested or anything. *We* do use them because *we* do like to. Anyway, I posted the flags I use more than one time on the list (not sure which one, maybe this one, maybe geany@uvena.de). Let's do it once more: http://nopaste.geany.org/p/fe14fca2
What exactly are you referring to by saying "some of which warn for perfectly conform C code". In case you are talking about the famous "don't mix code and variable declarations" warning, we could argue again. You would say, this is valid C and I would say this is not valid C. And we are both right as long as we don't specifiy about which C standard we are talking. Luckily, the HACKING file states the we want to stay compatible with C90 and developers can check their code with gcc's -ansi flag. Anyway, I really don't see a problem with this. Such things can be easily fixed and I didn't intend to really make this a big thing. If I did anyway, I'm sorry.
So, to finish of my mail: "like the rest of the code" is a poor and inaccurate statement. Besides, that this is basically true for any project I've seen, it doesn't help you identifying tiny stuff such as trying to avoid shadow
See above, such easy-to-fix compiler warnings are not a topic. I guess you are referring to the warning I was talking about in your patch when we discussed it on IRC, this was somewhat special because you modified a part of the code which was completely unrelated to your patch and this modification caused the warning. I just don't like doing many different, unrelated things in one patch. IMO patches should be rather atomic, similar to commits. But really, this is another story.
declarations or indentation when breaking long lines and stuff. HACKING should be as explicit as possible, to avoid additional work for *both*, the contributors and the reviewers.
I fully agree and will try to improve this soon. Sorry for the inconvenience you experienced in the meantime.
Regards, Enrico
On 08.09.2009 00:18, Enrico Tröger wrote:
On Mon, 07 Sep 2009 00:43:02 +0200, Thomas wrote:
Thomas Martitz schrieb:
Lex Trotman schrieb:
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code".
This is what I had an argument with Enrico too recently. HACKING is nowhere near accurate, as it's poorly describing what style Enrico
As already said, I try to update the HACKING file soon. Still, in the argument we had recently, I told you that some of the issues I had with your coding style we had already discussed in the past (e.g. in the snippets patches). But you are right, if the style guidelines would be better and more detailed described, we could have saved this conversion (and maybe even this one). It's somewhat my bad, no doubt.
In addition, seem Enrico and Nick run with extra CFLAGS which reveal extra warnings (some of which warn for perfectly conform C code, but well ;) ) which are not documented.
They are not "documented" because these flags are not required not suggested or anything. *We* do use them because *we* do like to. Anyway, I posted the flags I use more than one time on the list (not sure which one, maybe this one, maybe geany@uvena.de). Let's do it once more: http://nopaste.geany.org/p/fe14fca2
I think suggesting and documenting them officially would be a good idea.
What exactly are you referring to by saying "some of which warn for perfectly conform C code". In case you are talking about the famous "don't mix code and variable declarations" warning, we could argue again. You would say, this is valid C and I would say this is not valid C. And we are both right as long as we don't specifiy about which C standard we are talking. Luckily, the HACKING file states the we want to stay compatible with C90 and developers can check their code with gcc's -ansi flag. Anyway, I really don't see a problem with this. Such things can be easily fixed and I didn't intend to really make this a big thing. If I did anyway, I'm sorry.
Alright, I admit I knew about that guideline (and I know it's only valid C in C99). I tried hard to avoid having variable declarations after code, but I apparently failed (I can't remember now where it happened). That was my fault then.
See above, such easy-to-fix compiler warnings are not a topic. I guess you are referring to the warning I was talking about in your patch when we discussed it on IRC, this was somewhat special because you modified a part of the code which was completely unrelated to your patch and this modification caused the warning. I just don't like doing many different, unrelated things in one patch. IMO patches should be rather atomic, similar to commits. But really, this is another story.
It's not a problem at all. I'm perfectly fine for me if the coding style of geany is to avoid such shadow declarations, and I'll try to adhere to it. My point was, though, that if it was documented (maybe including the gcc option, since not everyone knows the bazillions of gcc options), it wouldn't have been a point of discussion in the first place :)
And yes, I know that part of the patch which introduced it was bad, I'm sorry for this. I would've taken it out in a new version, but then it was too late :)
Best regards.
declarations or indentation when breaking long lines and stuff. HACKING should be as explicit as possible, to avoid additional work for *both*, the contributors and the reviewers.
I fully agree and will try to improve this soon. Sorry for the inconvenience you experienced in the meantime.
Awesome. I'm sorry for the same reason.
On Mon, 7 Sep 2009 08:35:30 +1000 Lex Trotman elextr@gmail.com wrote:
How much of the changes are manual and how much is done by the
Dunno, Enrico?
fix_alignment script & is it safe to just run this every time pre-commit?
In the commit message IIRC I mentioned this is 'work in progress', I probably need to make some changes before this is reliable for all of src/ and plugins/.
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code". Then I could have saved you some work :-)
Well, we did tell you about a space after if and no space after an opening bracket ;-)
There is also the issue of a small amount of existing code sometimes being formatted wrongly, but hopefully with scripts we can iron out these.
At least the things not addressed by the script should be listed, eg when and if {} is used around single if clauses etc.
It seems we might need to write a clear style guide, although personally I think it's pretty easy to copy the existing code style. It would be quicker to read though.
Regards, Nick
2009/9/7 Nick Treleaven nick.treleaven@btinternet.com:
On Mon, 7 Sep 2009 08:35:30 +1000 Lex Trotman elextr@gmail.com wrote:
How much of the changes are manual and how much is done by the
Dunno, Enrico?
fix_alignment script & is it safe to just run this every time pre-commit?
In the commit message IIRC I mentioned this is 'work in progress', I probably need to make some changes before this is reliable for all of src/ and plugins/.
Sorry to jump the gun. I think I said in a previous post I advise my consulting clients to specify their coding standard by nominating a formatter and the options to be used. Other than picking a really stupid one, it doesn't matter what you actually pick so long as you always use it.
For those reading this and wondering why so much worry about coding style, humans are good at recognising patterns and notice divergences. When we are coding, debugging and maintaining code, if we are continually distracted by divergences from pattern due to coding style, we are less likely to notice the divergences due to real issues in the code.
So coding style *is* important and should be machine enforced, hence I think Nicks script is a great idea.
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code". Then I could have saved you some work :-)
Well, we did tell you about a space after if and no space after an opening bracket ;-)
I thought I had got most of them :-( but then I-am-not-a-mach-ine, oops that just slipped out, and I was passing the Turing test until then ;-)
There is also the issue of a small amount of existing code sometimes being formatted wrongly, but hopefully with scripts we can iron out these.
At least the things not addressed by the script should be listed, eg when and if {} is used around single if clauses etc.
It seems we might need to write a clear style guide, although personally I think it's pretty easy to copy the existing code style. It would be quicker to read though.
Its not always easy to determine whats important from examples of existing style or to correctly deduce it. For example I was *sure* you didn't put spaces around == and != although you did for other binary operators. I am not sure why I formed that conviction but clearly it was wrong.
Cheers Lex
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On 08.09.2009 01:54, Lex Trotman wrote:
For those reading this and wondering why so much worry about coding style, humans are good at recognising patterns and notice divergences. When we are coding, debugging and maintaining code, if we are continually distracted by divergences from pattern due to coding style, we are less likely to notice the divergences due to real issues in the code.
So coding style *is* important and should be machine enforced, hence I think Nicks script is a great idea.
I agree to all of that, except for the last part. I don't like these scripts, since teaching programmers in the first place on a good coding style in is the better way to enforce it. The learning curve on project-specific style is mostly quite short, while this scripts make programmers just careless about coding style in general (not project specific), and might be buggy at times messing up style everywhere.
It seems we might need to write a clear style guide, although personally I think it's pretty easy to copy the existing code style. It would be quicker to read though.
Its not always easy to determine whats important from examples of existing style or to correctly deduce it. For example I was *sure* you didn't put spaces around == and != although you did for other binary operators. I am not sure why I formed that conviction but clearly it was wrong.
Cheers Lex
That's true. There also might be totally tiny style characteristics which you don't even notice from reading existing code.
Best regards. http://dict.leo.org/ende?lp=ende&p=thMx..&search=characteristic
I agree to all of that, except for the last part. I don't like these scripts, since teaching programmers in the first place on a good coding style in is the better way to enforce it.
Oh well we'll have to agree to disagree on this (again :-)
I've never found manual style enforcement to be efficient or effective, & "teaching good style" comes back to personal decisions about what is "good".
The learning curve on
project-specific style is mostly quite short, while this scripts make programmers just careless about coding style in general (not project specific), and might be buggy at times messing up style everywhere.
Yes, normally I recommend using one of the mature formatters, even dear old GNU indent, since they are less likely to be buggy, but then turning off all the features you don't want is not always easy. Most give options like: for this code feature do this or do that but no option to do nothing. So sometimes limited scripts are the way to go, especially for projects with established styles that don't quite match the programs. In past experiments and discussions with Nick I never came up with one that quite matched Geany.
Cheers Lex
On Tue, 08 Sep 2009 02:28:16 +0200 Thomas Martitz thomas.martitz@student.HTW-Berlin.de wrote:
So coding style *is* important and should be machine enforced, hence I think Nicks script is a great idea.
I agree to all of that, except for the last part. I don't like these scripts, since teaching programmers in the first place on a good coding style in is the better way to enforce it. The learning curve on project-specific style is mostly quite short, while this scripts make programmers just careless about coding style in general (not project
The script is only intended to reformat some annoying but easy to script alignment, we still need to write the code style spec. If anyone wants to make a start on it... ;-)
specific), and might be buggy at times messing up style everywhere.
The script is incapable of fixing all but trivial things, and if it messes up anything the script must be fixed or not used.
Regards, Nick
On Thu, 10 Sep 2009 15:59:25 +0100 Nick Treleaven nick.treleaven@btinternet.com wrote:
we still need to write the code style spec. If anyone wants to make a start on it... ;-)
I just updated HACKING with more info on compiler warnings, code style and example code. (Web HTML should get auto-updated tonight sometime).
Regards, Nick
On Sun, 13 Sep 2009 16:39:42 +0100 Nick Treleaven nick.treleaven@btinternet.com wrote:
I just updated HACKING with more info on compiler warnings, code style and example code. (Web HTML should get auto-updated tonight sometime).
http://geany.org/manual/dev/hacking.html#coding
Regards, Nick
On Tue, 8 Sep 2009 09:54:11 +1000 Lex Trotman elextr@gmail.com wrote:
fix_alignment script & is it safe to just run this every time pre-commit?
In the commit message IIRC I mentioned this is 'work in progress', I probably need to make some changes before this is reliable for all of src/ and plugins/.
Sorry to jump the gun. I think I said in a previous post I advise my consulting clients to specify their coding standard by nominating a formatter and the options to be used. Other than picking a really stupid one, it doesn't matter what you actually pick so long as you always use it.
...
So coding style *is* important and should be machine enforced, hence I think Nicks script is a great idea.
It's ready for some testing, but there's still a few things I want to change before I run it on the codebase. No known bugs currently though. The idea is to only reformat things that unambiguously need it, there's no point introducing wrong formatting automatically, however small the manual fix is.
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code". Then I could have saved you some work :-)
Well, we did tell you about a space after if and no space after an opening bracket ;-)
I thought I had got most of them :-( but then I-am-not-a-mach-ine, oops that just slipped out, and I was passing the Turing test until then ;-)
Sorry, there probably were only a few left. It's more things like operator spacing, but the script should (mostly) take care of that.
Regards, Nick
On Mon, 7 Sep 2009 08:35:30 +1000, Lex wrote:
Heyho,
I'm somewhat embarrassed by the number of style fixes in build.c when
No need to. I didn't mean to show you up or something. I just wanted to make it more like I'm used to read our code :). It was really only because I wanted to do it and I took the time to. Nothing more, don't feel negative in any way. Your work is still much appreciated in the same way as before :).
How much of the changes are manual and how much is done by the
To be honest, I did it all manually. Actually, after I finished about 3/4 of the file, I remember Nick wrote a script for this...too late :).
Several of the things you changed I had thought I had right, so maybe hacking.html needs to be more explicit rather than just "like the rest of the code".
Surely. I will add a few things to the HACKING file soon, I hope.
Then I could have saved you some work :-)
No worries. The good thing is, we (me, Nick, you, everyone) can decide ourselves on what we spend our time and how much of it. So be sure, everything I do is what I want to do (sometimes for different reasons but still).
Regards, Enrico