Re: [HACKERS] Shapes on the regression test for polygon
On Tue, Oct 14, 2014 at 11:00:47AM -0400, Bruce Momjian wrote: On Tue, Oct 14, 2014 at 05:40:06PM +0300, Emre Hasegeli wrote: I extracted Emre's diagram adjustments from the patch and applied it, and no tabs now. Emre, I assume your regression changes did not affect the diagram contents. Thank you for looking at it. I wanted to make the tests consistent with the diagrams. Now they look better but they still don't make sense with the tests. I looked at it some more, and come to the conclusion that removing them is better than changing the tests. OK, unless I hear more feedback I will remove the diagrams. Removed. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
I extracted Emre's diagram adjustments from the patch and applied it, and no tabs now. Emre, I assume your regression changes did not affect the diagram contents. Thank you for looking at it. I wanted to make the tests consistent with the diagrams. Now they look better but they still don't make sense with the tests. I looked at it some more, and come to the conclusion that removing them is better than changing the tests. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
On Tue, Oct 14, 2014 at 05:40:06PM +0300, Emre Hasegeli wrote: I extracted Emre's diagram adjustments from the patch and applied it, and no tabs now. Emre, I assume your regression changes did not affect the diagram contents. Thank you for looking at it. I wanted to make the tests consistent with the diagrams. Now they look better but they still don't make sense with the tests. I looked at it some more, and come to the conclusion that removing them is better than changing the tests. OK, unless I hear more feedback I will remove the diagrams. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
On Wed, Jul 23, 2014 at 06:12:59PM -0400, Robert Haas wrote: On Wed, Jul 23, 2014 at 4:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli e...@hasegeli.com wrote: The first two shapes on src/test/regress/sql/polygon.sql do not make sense to me. Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. Perhaps we should expand tabs to spaces in those ascii-art diagrams? Personally, though, my vote would be to just leave it the way it is. I don't think there's really a problem here in need of solving. I concur with doubting that changing the actual regression test cases is a good thing to do, at least not without more analysis. But the pictures make no sense with the wrong tab setting is a pretty simple issue, and one that I can see biting people. AFAICT, the pictures make no sense with the right tab setting, either. The possibility that someone might use the wrong tab setting is just icing on the cake. I extracted Emre's diagram adjustments from the patch and applied it, and no tabs now. Emre, I assume your regression changes did not affect the diagram contents. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. Either way, I have no idea what the picture is supposed to mean, because it looks to me like the original picture has circles at (3,3) and (2,1) and plus signs at (2,2), (3,1), and (4,0). With your patch applied, the circle at (2,1) has moved to (2,0.5). I can't correlate any of this to the test cases you modified (and which I see no reason to modify, whether they match the picture or not). 4 space tab-stop is not the project standard? The circle I moved does not represent a corner of the polygon. I just moved it to make the line straight after the tabs. And really, if the diagram is confusing, let's just remove it. We don't really need our regression test comments to illustrate what a triangle looks like, or how to do bad ASCII art. Personally, though, my vote would be to just leave it the way it is. I don't think there's really a problem here in need of solving. I am sorry for taking your time for such a low priority problem, but as we stumble across this, let me to make the change more clear. According to git blame the tests with the shapes added by 04688df6. The coordinates was written in the old format like this: (3.0,3.0,1.0,1.0,3.0,0.0) These are changed by 3d9584c9 to the new format like this: (3.0,1.0),(3.0,3.0),(1.0,0.0) In my opinion, the real intention of the first commit was to write them in the new format like this: (3.0,3.0),(1.0,1.0),(3.0,0.0) It makes sense because the corners match the shape. So, I changed it that way. If we will not going to change the tests, It would be okay to just remove the shapes. I do not think they add more value to the tests. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
Emre Hasegeli e...@hasegeli.com writes: Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. 4 space tab-stop is not the project standard? It is for C code, but there's less agreement about non-code files (and no enforcement mechanism like pgindent, either). People may or may not have their editors configured to do the right thing in non-C files, so it seems best to avoid cases where it'd matter. We actually have a policy against using tabs at all in SGML files, for example. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli e...@hasegeli.com wrote: The first two shapes on src/test/regress/sql/polygon.sql do not make sense to me. They look more like polygons with some more tabs, but still did not match the coordinates. I changed them to make consistent with the shapes. I believe this was the intention of the original author. Patch attached. Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. Either way, I have no idea what the picture is supposed to mean, because it looks to me like the original picture has circles at (3,3) and (2,1) and plus signs at (2,2), (3,1), and (4,0). With your patch applied, the circle at (2,1) has moved to (2,0.5). I can't correlate any of this to the test cases you modified (and which I see no reason to modify, whether they match the picture or not). And really, if the diagram is confusing, let's just remove it. We don't really need our regression test comments to illustrate what a triangle looks like, or how to do bad ASCII art. Personally, though, my vote would be to just leave it the way it is. I don't think there's really a problem here in need of solving. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
Robert Haas robertmh...@gmail.com writes: On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli e...@hasegeli.com wrote: The first two shapes on src/test/regress/sql/polygon.sql do not make sense to me. Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. Perhaps we should expand tabs to spaces in those ascii-art diagrams? Personally, though, my vote would be to just leave it the way it is. I don't think there's really a problem here in need of solving. I concur with doubting that changing the actual regression test cases is a good thing to do, at least not without more analysis. But the pictures make no sense with the wrong tab setting is a pretty simple issue, and one that I can see biting people. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Shapes on the regression test for polygon
On Wed, Jul 23, 2014 at 4:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli e...@hasegeli.com wrote: The first two shapes on src/test/regress/sql/polygon.sql do not make sense to me. Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. Perhaps we should expand tabs to spaces in those ascii-art diagrams? Personally, though, my vote would be to just leave it the way it is. I don't think there's really a problem here in need of solving. I concur with doubting that changing the actual regression test cases is a good thing to do, at least not without more analysis. But the pictures make no sense with the wrong tab setting is a pretty simple issue, and one that I can see biting people. AFAICT, the pictures make no sense with the right tab setting, either. The possibility that someone might use the wrong tab setting is just icing on the cake. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
