draw
draw copied to clipboard
racket
Group color names together with alternative spellings, such as spaces/no-spaces and gray/grey
Instead of writing a bunch of colors like this:
...
("darkblue" . #(0 0 139))
("darkcyan" . #(0 139 139))
("darkgoldenrod" . #(184 134 11))
("darkgray" . #(169 169 169))
("darkgrey" . #(169 169 169))
...
("dark blue" . #(0 0 139))
("dark cyan" . #(0 139 139))
("dark goldenrod" . #(184 134 11))
("dark gray" . #(169 169 169))
("dark grey" . #(169 169 169))
...
They can now be written like this:
...
[(seq "dark" msp "blue") #(0 0 139)]
[(seq "dark" msp "cyan") #(0 139 139)]
[(seq "dark" msp "goldenrod") #(184 134 11)]
[(seq "dark" msp gray) #(169 169 169)]
...
Where msp stands for "maybe space" and is defined as (or " " ""). Similarly gray is used for the alternative spellings of gray, and is defined as (or "gray" "grey").
I don't think we can change the set of names allowed here. But I was planning to change the set for 2htdp/image.
Robby
On Sat, Jan 19, 2019 at 6:04 PM Alex Knauth [email protected] wrote:
Instead of writing a bunch of colors like this:
... ("darkblue" . #(0 0 139)) ("darkcyan" . #(0 139 139)) ("darkgoldenrod" . #(184 134 11)) ("darkgray" . #(169 169 169)) ("darkgrey" . #(169 169 169)) ... ("dark blue" . #(0 0 139)) ("dark cyan" . #(0 139 139)) ("dark goldenrod" . #(184 134 11)) ("dark gray" . #(169 169 169)) ("dark grey" . #(169 169 169)) ...
They can now be written like this:
... [(seq "dark" msp "blue") #(0 0 139)] [(seq "dark" msp "cyan") #(0 139 139)] [(seq "dark" msp "goldenrod") #(184 134 11)] [(seq "dark" msp gray) #(169 169 169)] ...
Where msp stands for "maybe space" and is defined as (or " " ""). Similarly gray is used for the alternative spellings of gray, and is defined as (or "gray" "grey").
You can view, comment on, or merge this pull request online at:
https://github.com/racket/draw/pull/16 Commit Summary
- add section separator
- refator to use define-colors helper macro
- use name-groups to merge variations of spaces and alt-spellings together
File Changes
- M draw-lib/racket/draw/private/color.rkt https://github.com/racket/draw/pull/16/files#diff-0 (414)
Patch Links:
- https://github.com/racket/draw/pull/16.patch
- https://github.com/racket/draw/pull/16.diff
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/racket/draw/pull/16, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYWsFddfg-RXbq02lZfYmA6N7yXOMvaks5vE7J1gaJpZM4aJZRJ .
I think they are from various standards but I do not recall exactly. I tried to search on the web but didn't get a lot of clarity.
Robby
On Sat, Jan 19, 2019 at 6:58 PM Alex Knauth [email protected] wrote:
Is there a reason why not?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/racket/draw/pull/16#issuecomment-455828144, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYWsIO1-u6CyVP1MAUP9cZPyMdkscH8ks5vE78cgaJpZM4aJZRJ .
I think so but I am not sure. I think the the X11 spec is where they came from and maybe it is in CSS too.
Robby
On Sat, Jan 19, 2019 at 7:10 PM Alex Knauth [email protected] wrote:
Are the variations in spaces vs. no spaces from those standards?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/racket/draw/pull/16#issuecomment-455828674, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYWsLEt7o8gtNUpvVBnNwSsZSqgUUZTks5vE8HygaJpZM4aJZRJ .
The HTML/CSS color names include some alternative spellings, like both "gray" and "grey":
https://www.w3schools.com/colors/colors_names.asp
This wikipedia page seems to suggest that the color names in the X11 list (which is, I believe, where we got them) made their way into the CSS and SVG specifications. There are spaces in that list, but no with-space/without-space duplicates that I see.
Here is the w3c color spec, see the "4.3. Extended color keywords" section. It adds the gray/grey compared to the other (according to the text there; I didn't check the lists myself).
It looks like SVG 1.2 is the latest SVG spec and its color section points to SVG 1.1 spec's color keywords. If I am reading the SVG spec properly, no spaces are allowed in the color names.
And to make things yet more complicated, there's also a long list of colors here.
In short, it seems like adding colors so that grey and gray all work has some precedent so perhaps we should consider it. Also it seems like the choices about where the spaces go is somewhat arbitrary, so maybe we should list them in the docs with spaces in "reasonable" places, but say that spaces are removed from the names before looking them up in the database (and then do that).
Not that I'm entirely sure about those two specific recommendations, but I'm certainly much less sure that we should try to add the list of colors in the long list of colors.
Probably it would also be good to actually go over those four lists (ours, X11, SVG, CSS) and figure out what the actual differences are, if any.
Okay. It does make more sense to remove spaces from names before looking them up in the hash table, rather than have multiple copies of an entry in the hash table. The find-color method already calls string-downcase on the name before looking it up. It makes sense to normalize by remove spaces in the same place.
FWIW, some internal history to add to Robby's thorough external history: The list in our implementation originally came from wxWindows. At that time, the numbers varied across platforms, and essentially the same list turned into a web de facto standard, so I tried to sync everything up (especially the numbers) and declare the list wouldn't change. I left the extra names with spaces in as a compromise to avoid breaking old programs. It looks like "grey" names were added to standards after I synced the colors, but I'm unsure.
I worry about changing anything here and my inclination would be to leave well enough alone. But it does sound ok to remove spaces on lookup strings and add "grey".
As far as I know, we have no compelling reason to change these colors. Already, the 2htdp/image color names are different and we can add the alternative grays there, which would have solved the original symptom behind this pull request. (I'm skeptical of changes that don't have a clear rationale, preferring to leave things alone if possible/reasonable.)
That said, pushing the changes that adjusts the implementation to be clearly in the cool way Alex suggests seems like a very nice win.
I have made https://github.com/racket/draw/pull/17, which just normalizes the spaces. I'm thinking of rebasing and modifying this pull request to only deal with adding {gray, grey} to every version of gray, since the spaces would be covered by https://github.com/racket/draw/pull/17, but I won't get to that until this evening.
@AlexKnauth if I understand the consensus correctly here, it is to make no observable changes to the color names (but that cleaning up the code in the way you wrote looks quite cute). Are you saying you wish us to reach a different consensus?
consensus is to make no observable changes to the color names
Are you referring to #17 changing the behavior of "cadet blue" and "cornflower blue" to match the no-space versions "cadetblue" and "cornflowerblue"? Are you referring to #17 changing get-names? Or both, or something else?
It seems like "cadetblue" and "cadet blue" were meant to be the same color, and if we're going to say that spaces don't matter anymore, then they should be the same color. The "cornflower blue" color values seem like a mistake to me. It looks way to dark to be a cornflower blue, so seems to me that it should be changed to be the same as "cornflowerblue".
On get-names, I guess I could leave those extra entries in the hash-table. They would never get used for lookup, but leaving them there does preserve the behavior of get-names. Is that what you want?
Please don't change the color of "cornflower blue", even if it seems too dark.
IIUC, that's what Robby & Matthew are asking too --- don't make changes that would change the output of existing programs (unless there is a very good reason for doing so).
Right. What Ben says. I can believe these things are bugs but others probably have no conception of what "cornflower blue" is supposed to be and just looked at the chart and picked a color they liked. (People like me ...)
Robby
On Mon, Jan 21, 2019 at 10:22 PM Ben Greenman [email protected] wrote:
Please don't change the color of "cornflower blue", even if it seems too dark.
IIUC, that's what Robby & Matthew are asking too --- don't make changes that would change the output of existing programs (unless there is a very good reason for doing so).
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/racket/draw/pull/16#issuecomment-456266667, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYWsCW143tkyUzJ_QfrCf8WOp36IAsUks5vFpIBgaJpZM4aJZRJ .
So to normalize a color name, downcase it, and then check whether that version is equal to "cadet blue" or "cornflower blue". If it is, then stop there. Otherwise, remove spaces.
;; normalize-color-name : String -> String
;; Normalizes a color name to the form used for keys in the `colors`
;; table. This includes converting to lowercase and removing spaces.
;; Except for "cadet blue" and "cornflower blue", those are different
;; colors from "cadetblue" and "cornflowerblue" respectively.
(define (normalize-color-name name)
(define lower (string-downcase name))
(cond [(member lower '("cadet blue" "cornflower blue")) lower]
[else (regexp-replace* #px"\\s+" lower "")]))
This preserves existing behavior, so even if it looks ugly to me, I'll change it.
This doesn't seem to preserve existing behavior to me. It accepts things with multiple spaces and spaces in strange places.
Robby
On Tue, Jan 22, 2019 at 8:27 AM Alex Knauth [email protected] wrote:
So to normalize a color name, downcase it, and then check whether that version is equal to "cadet blue" or "cornflower blue". If it is, then stop there. Otherwise, remove spaces.
;; normalize-color-name : String -> String;; Normalizes a color name to the form used for keys in the
colors;; table. This includes converting to lowercase and removing spaces.;; Except for "cadet blue" and "cornflower blue", those are different;; colors from "cadetblue" and "cornflowerblue" respectively. (define (normalize-color-name name) (define lower (string-downcase name)) (cond [(member lower '("cadet blue" "cornflower blue")) lower] [else (regexp-replace* #px"\s+" lower "")]))This preserves existing behavior, so even if it looks ugly to me, I'll change it.
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/racket/draw/pull/16#issuecomment-456418299, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYWsKjW56llqWnQAf_mH1Xp-3THSt7lks5vFx_QgaJpZM4aJZRJ .
Reading over the discussion here it seems like there was some confusion on the question of whether or not it was acceptable to have something like: (send the-color-database find-color "b l u e") change from returning #f to returning the same thing as (send the-color-database find-color "blue"). I understood that this was NOT acceptable but I may have been wrong about that. @AlexKnauth seemed to believe that that was okay and desirable. @bennn and @mflatt what do you think?
Normalizing "b l u e" to "blue" is ok with me, as long as old colors have the same behavior. I think #17 did that, and this PR is only about how the source code looks.
I haven't read this PR carefully, but I can if that'd be helpful.