Re: [PATCH] FCI work for cabinet.dll [cabinet-fci-patch-02b.diff]
From: "Gerold J. Wucherpfennig" <gjwucherpfennig(a)gmx.net> Cool stuff Gerold. A few nits:
+ if(PFCI_INT(hfci)->fNextCab==TRUE) {
It's better to avoid explicit comparison with TRUE. What if fNextCab is 10, not 1? It's still true, but it will fail the above test. Just do: + if(PFCI_INT(hfci)->fNextCab) { Also, I know you're modifying existing code, but if it's not too much to ask, 2-space indent is hard to read, and the vast majority of developers do prefer 4-space indent. So if you don't mind, and we can find a solution, would be great if we can stick to 4-spaces instead. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
On Wednesday 08 June 2005 18:16, Dimi Paun wrote:
Also, I know you're modifying existing code, but if it's not too much to ask, 2-space indent is hard to read, and the vast majority of developers do prefer 4-space indent. So if you don't mind, and we can find a solution, would be great if we can stick to 4-spaces instead.
Probably because I put all the cabinet.dll sources this way, as I am a two-space freak (and maybe because the code had too much nesting for four? Also it might have been this way when I cut-and-pasted it :P ) -- gmt
On Wednesday 08 June 2005 19:16, you wrote:
From: "Gerold J. Wucherpfennig" <gjwucherpfennig(a)gmx.net>
Cool stuff Gerold. A few nits:
+ if(PFCI_INT(hfci)->fNextCab==TRUE) {
It's better to avoid explicit comparison with TRUE. What if fNextCab is 10, not 1? It's still true, but it will fail the above test. Just do:
+ if(PFCI_INT(hfci)->fNextCab) {
Hi Dimi, actually I don't who you are and how involved you are in the wine development process. I have to admit that I'm just a beginner wine-dev-wise, but I have a strong opinion about the use of BOOL. In my opinion BOOL is a crap and a real shortcoming of the C programming language. To be on the "super-safe" side one has to check before a BOOL value is checked if the value is different from TRUE and FALSE. When that's the case bail out the running program with an INTERNAL ERROR. That would make the source code hartly readable so I won't do that. (Remember: It's better to break a programm than to corrupt it.) A BOOL value should never be assigned a value other than TRUE or FALSE. Use the ? : operator if necessary. (If anybody disagrees the one should undefine TRUE and use !=FALSE instead for the sake of integrity) And last: if(fNextCab==TRUE) makes it more obvious for the quick source code reader when a BOOL value is compared than if(fNextCab).
Also, I know you're modifying existing code, but if it's not too much to ask, 2-space indent is hard to read, and the vast majority of developers do prefer 4-space indent. So if you don't mind, and we can find a solution, would be great if we can stick to 4-spaces instead.
Sorry Dimi, my opinion differs here too... 1. I don't want to type the spacebar 4 times an ident is made (though I could configure the text editor to do "this" for me). 2. I want to stick to the maximum column width of 80 chars. With 5 idents in a line that would only left 60 chars of code compared to 70. 3. In my opinion with an ident of 2 chars you can clearly distinguish two diffent idents though my eyes are really not the best, so an ident of 4 chars is unnecessary. 4. I don't want to use 4-space idents if there are other wine developers who also use differing idents, because I not convinced of 4-space idents. By the way I've fixed some of the remaining bugs, but I will post a new patch when I've fixed all known bugs, so please do not commit old patches. Regards, Gerold
On Sat, 2005-06-11 at 16:37 +0200, Gerold J. Wucherpfennig wrote:
In my opinion BOOL is a crap and a real shortcoming of the C programming language. [...] And last: if(fNextCab==TRUE) makes it more obvious for the quick source code reader when a BOOL value is compared than if(fNextCab).
Well, like it or not, that's the semantics in C. This is not a matter of preference like the indentation, but a matter of potentially introducing very hard to find bugs. Also remember that in many cases, we are dealing with values that are set by the application, and are not under our control. For those we must not do fBool == TRUE. It would be confusing to have two very different ways to test for TRUE. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
Ok, Now I am getting fed up with all this petty bickering over indent amounts. Alexandre wants 4 space indents. As soon as I get Linux installed on my box, I'm going to _MANUALLY_ go through every single file and make sure that we have 4 space indent, since I wouldnt know how to write up a script to do it automatically.. Then I'm going to make diffs and submit patches. I honestly prefer 2-space, but at the same time, I can understand why most devs prefer 4 space. My question is though, how long will it take once the changes have been made before the code is all mixed up again? Is there any way we can just not accept patches that dont conform? Of course if we do that, there would be some angry devs, so maybe a compromise for this would be to accept the patch but run a script to auto format it to 4 space before it gets committed to CVS?? Dustin Gerold J. Wucherpfennig wrote:
Sorry Dimi, my opinion differs here too...
1. I don't want to type the spacebar 4 times an ident is made (though I could configure the text editor to do "this" for me). 2. I want to stick to the maximum column width of 80 chars. With 5 idents in a line that would only left 60 chars of code compared to 70. 3. In my opinion with an ident of 2 chars you can clearly distinguish two diffent idents though my eyes are really not the best, so an ident of 4 chars is unnecessary. 4. I don't want to use 4-space idents if there are other wine developers who also use differing idents, because I not convinced of 4-space idents.
On Sat, 2005-06-11 at 21:55 -0500, Dustin Navea wrote:
Ok, Now I am getting fed up with all this petty bickering over indent amounts. Alexandre wants 4 space indents. As soon as I get Linux installed on my box, I'm going to _MANUALLY_ go through every single file and make sure that we have 4 space indent, since I wouldnt know how to write up a script to do it automatically.
Don't worry about it: many people prefer 4 space indents, but Alexandre has made it clear that the developer is free to pick whatever indent they like for new code. So he's not going to accept patches that just reindent stuff, and rightfully so. Now, a lot of people don't feel strongly about it, and we just have a recommendation that 4-space indent is used to make it pleasant for others to work with that code as well. If the developer however feels very strongly about it, then they can choose whatever makes them happy (within reason, of course). -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
OHHH.. Boy do I feel a lil silly now. I was thinking it was pretty much a steadfast rule, but that if someone felt strongly about their indentation style, we would still accept their patch so that we could get more functionality, and at the same time encourage them to submit more.. Guess I was wrong. Sorry everyone for my (almost) flame.. Dustin Dimi Paun wrote:
On Sat, 2005-06-11 at 21:55 -0500, Dustin Navea wrote:
Ok, Now I am getting fed up with all this petty bickering over indent amounts. Alexandre wants 4 space indents. As soon as I get Linux installed on my box, I'm going to _MANUALLY_ go through every single file and make sure that we have 4 space indent, since I wouldnt know how to write up a script to do it automatically.
Don't worry about it: many people prefer 4 space indents, but Alexandre has made it clear that the developer is free to pick whatever indent they like for new code. So he's not going to accept patches that just reindent stuff, and rightfully so.
Now, a lot of people don't feel strongly about it, and we just have a recommendation that 4-space indent is used to make it pleasant for others to work with that code as well. If the developer however feels very strongly about it, then they can choose whatever makes them happy (within reason, of course).
Dustin Navea wrote:
Ok, Now I am getting fed up with all this petty bickering over indent amounts. Alexandre wants 4 space indents. As soon as I get Linux installed on my box, I'm going to _MANUALLY_ go through every single file and make sure that we have 4 space indent, since I wouldnt know how to write up a script to do it automatically.. Then I'm going to make diffs and submit patches. I honestly prefer 2-space, but at the same time, I can understand why most devs prefer 4 space. My question is though, how long will it take once the changes have been made before the code is all mixed up again? Is there any way we can just not accept patches that dont conform? Of course if we do that, there would be some angry devs, so maybe a compromise for this would be to accept the patch but run a script to auto format it to 4 space before it gets committed to CVS??
Dustin
Gerold J. Wucherpfennig wrote:
Sorry Dimi, my opinion differs here too...
1. I don't want to type the spacebar 4 times an ident is made (though I could configure the text editor to do "this" for me). 2. I want to stick to the maximum column width of 80 chars. With 5 idents in a line that would only left 60 chars of code compared to 70. 3. In my opinion with an ident of 2 chars you can clearly distinguish two diffent idents though my eyes are really not the best, so an ident of 4 chars is unnecessary. 4. I don't want to use 4-space idents if there are other wine developers who also use differing idents, because I not convinced of 4-space idents.
.
I've always wondered, why not tabs? Any decent text editor can display tabs however wide someone wants them to be. Wouldn't that solve the whole problem? Or is there some other problem with tabs that I'm missing? --Mitchell Mebane -- I find that a great part of the information I have was acquired by looking up something and finding something else on the way. -- Franklin P. Adams
On Sun, 2005-06-12 at 10:03 -0500, Mitchell Mebane wrote:
I've always wondered, why not tabs? Any decent text editor can display tabs however wide someone wants them to be. Wouldn't that solve the whole problem? Or is there some other problem with tabs that I'm missing?
In fact, if editors wouldn't have allowed such a thing, we would be in a much saner state now. Why not tabs: -- the standard is 8 -- many common editors do not support tab redefinition (e.g. notepad) -- tools assume 8 (cvsweb, viewcvs, etc) -- printers always print it as 8 -- you don't want to setup your editor before actually doing work -- if people mix tabs and spaces (and they do) the output is not sane Tab is 8 spaces. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
On 6/12/05, Dimi Paun <dimi(a)lattica.com> wrote:
-- if people mix tabs and spaces (and they do) the output is not sane
Tab is 8 spaces.
Why not go from 4-space indent as a request to a rule? And then in the future you can simple say this is the rule of coding etiquette here? Tom
-- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
Why not a compromise, then ? I'm sure three space indents are clear enough, but do not involve excessive typing. (IIRC, M$ and Borland both use 3-space) Andrew You can be the captain I will draw the chart Sailing into destiny Closer to the heart Closer to the Heart by Rush (A Farewell to Kings, 1977) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Tom Wickline wrote:
On 6/12/05, Dimi Paun <dimi(a)lattica.com> wrote:
-- if people mix tabs and spaces (and they do) the output is not sane
Tab is 8 spaces.
Why not go from 4-space indent as a request to a rule? And then in the future you can simple say this is the rule of coding etiquette here?
Tom
-- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
On Sun, 2005-06-12 at 21:12 +0100, Andrew Neil Ramage wrote:
Why not a compromise, then ? I'm sure three space indents are clear enough, but do not involve excessive typing. (IIRC, M$ and Borland both use 3-space)
:) No, please don't. In the Wine tree we currently use 2, 4, 8 space indents. I sure this is divers enough that you can pick one of the above and be (reasonably) happy with it. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
participants (7)
-
Andrew Neil Ramage -
Dimi Paun -
Dustin Navea -
Gerold J. Wucherpfennig -
Gregory M. Turner -
Mitchell Mebane -
Tom Wickline