* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs [not found] ` <20220306130800.EE0F9C01681@vcs2.savannah.gnu.org> @ 2022-03-06 13:46 ` Po Lu 2022-03-06 13:56 ` Mattias Engdegård 0 siblings, 1 reply; 12+ messages in thread From: Po Lu @ 2022-03-06 13:46 UTC (permalink / raw) To: emacs-devel; +Cc: Mattias Engdegård Mattias Engdegård <mattiase@acm.org> writes: > branch: master > commit 6eeab90632506348a58d264eb3304625756c8659 > Author: Mattias Engdegård <mattiase@acm.org> > Commit: Mattias Engdegård <mattiase@acm.org> > > Don't accept whitespace or hex floats in rgbi: colour specs > > `color-values-from-color-spec` (new in Emacs 28) erroneously accepted > leading whitespace and hex floats in rgbi: components. > > Reported by Philip Kaludercic. > > * src/xfaces.c (parse_float_color_comp): Disallow leading whitespace > and hex floats. > * test/src/xfaces-tests.el > (xfaces-internal-color-values-from-color-spec): Add test cases. I don't see a corresponding bug report, so please forgive me if I missed something here. If the leading whitespace and hex floats don't cause problems, why not keep the ability to understand them? It could come in handy. Thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-06 13:46 ` master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs Po Lu @ 2022-03-06 13:56 ` Mattias Engdegård 2022-03-06 16:49 ` [External] : " Drew Adams 2022-03-07 0:34 ` Po Lu 0 siblings, 2 replies; 12+ messages in thread From: Mattias Engdegård @ 2022-03-06 13:56 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel 6 mars 2022 kl. 14.46 skrev Po Lu <luangruo@yahoo.com>: > I don't see a corresponding bug report, so please forgive me if I missed > something here. It's bug#54263 -- sorry about the omission from the log message. > If the leading whitespace and hex floats don't cause problems, why not > keep the ability to understand them? It could come in handy. So it might seem, but over-tolerant implementation is generally a mistake. It restricts compatibility in the other direction (what worked with Emacs won't work elsewhere), restricts future changes and implementations, detects fewer user mistakes, and makes documentation and specifications less useful. It's not doing the user a service. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [External] : Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-06 13:56 ` Mattias Engdegård @ 2022-03-06 16:49 ` Drew Adams 2022-03-07 0:34 ` Po Lu 1 sibling, 0 replies; 12+ messages in thread From: Drew Adams @ 2022-03-06 16:49 UTC (permalink / raw) To: Mattias Engdegård, Po Lu; +Cc: emacs-devel@gnu.org > > If the leading whitespace and hex floats don't cause > > problems, why not keep the ability to understand them? > > It could come in handy. > > So it might seem, but over-tolerant implementation is generally a > mistake. It restricts compatibility in the other direction (what worked > with Emacs won't work elsewhere), restricts future changes and > implementations, detects fewer user mistakes, and makes documentation > and specifications less useful. It's not doing the user a service. +1. Dunno about this particular case, but there are often (at least) two different use cases for such things. They could both be accommodated easily by adding an optional arg, LAX, or its opposite, STRICT, to choose the other behavior. The two behaviors are: 1. LAX: Allow nonexact, nonkosher, or in some way looser matches. 2. STRICT: Allow only exact matches. Examples: 1. JSON - in practice - in the wild, lots of nonstandard syntax is used. Applications often want to support such syntax because, well, it's used - common. But if you want/need to know _whether_ some data is well-formed JSON data according to the standard, then you don't want to allow data that's not well-formed. https://docs.oracle.com/en/database/oracle/oracle-database/21/adjsn/conditions-is-json-and-is-not-json.html#GUID-1B6CFFBE-85FE-41DD-BA14-DD1DE73EAB20 (There's also the notion of "validity", relative to a JSON schema. That's different; there, the schema decides what's allowed.) 2. Thing-at-point - Emacs (unfortunately) doesn't test for a THING being _at_ point. It instead tests for a THING being either at point or at (1- point). This could be considered a good thing in contexts where all you want is to maximize the chances of obtaining a thing from the buffer for use as, say, a default value for input. But it's definitely a bad thing if what you want is to actually find out if there is a THING at point - that is, if you really want a THING only if it's AT point. My own approach is to have thing-at-point and its bounds actually check for a thing _AT_ point, and to provide other functions for obtaining a THING that's _near_ point, where you can control what "near" means. See bug #9300: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=9300 https://www.emacswiki.org/emacs/ThingAtPointPlus 3. Completion - If your aim is to maximize the number of possible completion candidates, then a very loose kind of matching can be good. But if you want to obtain a set of candidates that are particularly appropriate, then such looseness can just introduce noise. This is a problem with the `completion-styles' design of Emacs. The fallback process, from a failing style to the next and so on, is really aimed at maximizing the chances of finding a candidate of some kind (the styles define the possible kinds - the matching is a union/OR of all of those kinds). Your code can't know which kind of matching was used successfully. My own approach lets users change completion methods on the fly, that is, control their use as _alternatives_. Then you know what kind of matching is used and is successful or not. (I also support using `completion-styles', in combination with other methods. But you're not a captive of the fallback behavior that `completion-styles' design locks you into.) https://www.emacswiki.org/emacs/Icicles_-_Completion_Methods_and_Styles#VanillaEmacsStylesAndOption_completing-styles ______________________ You get the point. There are advantages to both LAX and STRICT. (And there can be more complex division/choices than just a binary LAX/STRICT.) A LAX choice can help if the priority is to get (or support) _something_, rather than raising an error or returning nil or NULL. A STRICT choice is useful if you want to know _whether_ there's a match, and not just obtain _something_. ___ Without bothering to understand more about this RGBI thing, a naive suggestion would be to add an optional STRICT arg. That would leave the behavior as it is now, by default (so backward compatible, if that's important). But it would also let you test for an exact match. (If this is a new feature, so there's no need to keep the existing behavior by default, then you could change it to be STRICT by default and accept an optional LAX arg.) ___ We should keep this kind of thing in mind. Pretty much any time there's some matching or built-in predicate-satisfying involved, it's worth thinking about whether it makes sense to either add an optional argument to provide either (1) a common "opposite" behavior (LAX or STRICT) or (2) a predicate, or other function, to specify the behavior (overriding the default behavior). This hopefully all seems obvious. But I think it's not always kept in mind. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-06 13:56 ` Mattias Engdegård 2022-03-06 16:49 ` [External] : " Drew Adams @ 2022-03-07 0:34 ` Po Lu 2022-03-07 9:26 ` Philip Kaludercic 2022-03-07 9:31 ` Mattias Engdegård 1 sibling, 2 replies; 12+ messages in thread From: Po Lu @ 2022-03-07 0:34 UTC (permalink / raw) To: Mattias Engdegård; +Cc: emacs-devel Mattias Engdegård <mattiase@acm.org> writes: > It's bug#54263 -- sorry about the omission from the log message. Thanks. > So it might seem, but over-tolerant implementation is generally a > mistake. It restricts compatibility in the other direction (what > worked with Emacs won't work elsewhere) AFAICT, there are no other implementations of such an rgbi color specification (please correct me if I'm wrong.) > restricts future changes and implementations, detects fewer user > mistakes, and makes documentation and specifications less useful. It's > not doing the user a service. I don't think there's a chance that the user will type any of those by mistake. Besides, the documentation can always be amended, and that function has no other specification. So I disagree that it doesn't do the user a service. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 0:34 ` Po Lu @ 2022-03-07 9:26 ` Philip Kaludercic 2022-03-07 10:16 ` Po Lu 2022-03-07 9:31 ` Mattias Engdegård 1 sibling, 1 reply; 12+ messages in thread From: Philip Kaludercic @ 2022-03-07 9:26 UTC (permalink / raw) To: Po Lu; +Cc: Mattias Engdegård, emacs-devel Po Lu <luangruo@yahoo.com> writes: > Mattias Engdegård <mattiase@acm.org> writes: > >> It's bug#54263 -- sorry about the omission from the log message. > > Thanks. > >> So it might seem, but over-tolerant implementation is generally a >> mistake. It restricts compatibility in the other direction (what >> worked with Emacs won't work elsewhere) > > AFAICT, there are no other implementations of such an rgbi color > specification (please correct me if I'm wrong.) It seems the "rgbi" (that stands for RGB Intensity) syntax comes from X11[0], specifically "XCMS" (X Color Management System) Interestingly enough, if you look around you can find that the "rgbi" prefix seems to be case-insensitive[1], while `color-values-from-color-spec' is not: (color-values-from-color-spec "rgbi:0/0/0") ;; => (0 0 0) (color-values-from-color-spec "rgbI:0/0/0") ;; => nil (color-values-from-color-spec "RGBi:0/0/0") ;; => nil (color-values-from-color-spec "RGBI:0/0/0") ;; => nil The closes thing to a specification I could find was [2], that says: 1.3.2.2. RGB Intensity String Specification An RGB intensity specification in the form of a string is identified by the prefix "rgbi:" and whose string conforms to the following syntax: rgbi:<red>/<green>/<blue> Where red, green, and blue are floating point values between 0.0 and 1.0, inclusive. The input format for these values is an optional sign, a string of numbers possibly containing a decimal point, and an optional exponent field containing an E or e followed by a possibly signed integer string. [0] https://man.archlinux.org/man/extra/libx11/XLookupColor.3.en. [1] https://www.niksula.hut.fi/~jkirma/books/xlib.pdf, section 7.2.2, https://www.klauser.ch/lxug/ch14.pdf, page 18. [2] http://www.nic.funet.fi/pub/X11/X11R4/DOCS/color/Xcms.text -- Philip Kaludercic ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 9:26 ` Philip Kaludercic @ 2022-03-07 10:16 ` Po Lu 2022-03-07 15:16 ` Philip Kaludercic 0 siblings, 1 reply; 12+ messages in thread From: Po Lu @ 2022-03-07 10:16 UTC (permalink / raw) To: Philip Kaludercic; +Cc: Mattias Engdegård, emacs-devel Philip Kaludercic <philipk@posteo.net> writes: > It seems the "rgbi" (that stands for RGB Intensity) syntax comes from > X11[0], specifically "XCMS" (X Color Management System) Interestingly > enough, if you look around you can find that the "rgbi" prefix seems > to be case-insensitive[1], while `color-values-from-color-spec' is not: > > (color-values-from-color-spec "rgbi:0/0/0") ;; => (0 0 0) > (color-values-from-color-spec "rgbI:0/0/0") ;; => nil > (color-values-from-color-spec "RGBi:0/0/0") ;; => nil > (color-values-from-color-spec "RGBI:0/0/0") ;; => nil > > The closes thing to a specification I could find was [2], that says: > > 1.3.2.2. RGB Intensity String Specification > > An RGB intensity specification in the form of a string is > identified by the prefix "rgbi:" and whose string conforms > to the following syntax: > > rgbi:<red>/<green>/<blue> > > Where red, green, and blue are floating point values between > 0.0 and 1.0, inclusive. The input format for these values > is an optional sign, a string of numbers possibly containing > a decimal point, and an optional exponent field containing > an E or e followed by a possibly signed integer string. See my reply to Mattias, that's not the RGBi specification I'm talking about. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 10:16 ` Po Lu @ 2022-03-07 15:16 ` Philip Kaludercic 0 siblings, 0 replies; 12+ messages in thread From: Philip Kaludercic @ 2022-03-07 15:16 UTC (permalink / raw) To: Po Lu; +Cc: Mattias Engdegård, emacs-devel Po Lu <luangruo@yahoo.com> writes: > Philip Kaludercic <philipk@posteo.net> writes: > >> It seems the "rgbi" (that stands for RGB Intensity) syntax comes from >> X11[0], specifically "XCMS" (X Color Management System) Interestingly >> enough, if you look around you can find that the "rgbi" prefix seems >> to be case-insensitive[1], while `color-values-from-color-spec' is not: >> >> (color-values-from-color-spec "rgbi:0/0/0") ;; => (0 0 0) >> (color-values-from-color-spec "rgbI:0/0/0") ;; => nil >> (color-values-from-color-spec "RGBi:0/0/0") ;; => nil >> (color-values-from-color-spec "RGBI:0/0/0") ;; => nil >> >> The closes thing to a specification I could find was [2], that says: >> >> 1.3.2.2. RGB Intensity String Specification >> >> An RGB intensity specification in the form of a string is >> identified by the prefix "rgbi:" and whose string conforms >> to the following syntax: >> >> rgbi:<red>/<green>/<blue> >> >> Where red, green, and blue are floating point values between >> 0.0 and 1.0, inclusive. The input format for these values >> is an optional sign, a string of numbers possibly containing >> a decimal point, and an optional exponent field containing >> an E or e followed by a possibly signed integer string. > > See my reply to Mattias, that's not the RGBi specification I'm talking > about. The point remains that if rgbi: is supported, it should be case-insensitive. -- Philip Kaludercic ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 0:34 ` Po Lu 2022-03-07 9:26 ` Philip Kaludercic @ 2022-03-07 9:31 ` Mattias Engdegård 2022-03-07 10:14 ` Po Lu 1 sibling, 1 reply; 12+ messages in thread From: Mattias Engdegård @ 2022-03-07 9:31 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel 7 mars 2022 kl. 01.34 skrev Po Lu <luangruo@yahoo.com>: > AFAICT, there are no other implementations of such an rgbi color > specification (please correct me if I'm wrong.) The syntax is from X11, or at least that is why Emacs supports it. > I don't think there's a chance that the user will type any of those by > mistake. Besides, the documentation can always be amended, and that > function has no other specification. So I disagree that it doesn't do > the user a service. These unintended extensions do not provide the user with any additional expressive power so there seems to be little reason for them. For that matter, I believe that extensions should be motivated by needs, not by implementation happenstance. Don't you agree? In any case we are debating angels dancing on a pinhead. I just fixed it because I made the mistake to begin with, and I don't think you should elevate my errors to holy scripture quite yet (please wait until I'm dead). ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 9:31 ` Mattias Engdegård @ 2022-03-07 10:14 ` Po Lu 2022-03-07 13:36 ` Mattias Engdegård 2022-03-07 16:06 ` [External] : " Drew Adams 0 siblings, 2 replies; 12+ messages in thread From: Po Lu @ 2022-03-07 10:14 UTC (permalink / raw) To: Mattias Engdegård; +Cc: emacs-devel Mattias Engdegård <mattiase@acm.org> writes: > The syntax is from X11, or at least that is why Emacs supports it. But the X11 intensity string specification doesn't accept whitespace or hexidecimal values, so it's different from the rgbi specification that was implemented in Emacs. Though in practice, tolerance for whitespace is X server-specific. For the same reason, we don't use XParseColor to parse color specifications. > These unintended extensions do not provide the user with any > additional expressive power so there seems to be little reason for > them. For that matter, I believe that extensions should be motivated > by needs, not by implementation happenstance. Don't you agree? If the happenstance doesn't cause trouble, there is no need to get rid of it. Thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 10:14 ` Po Lu @ 2022-03-07 13:36 ` Mattias Engdegård 2022-03-07 13:52 ` Po Lu 2022-03-07 16:06 ` [External] : " Drew Adams 1 sibling, 1 reply; 12+ messages in thread From: Mattias Engdegård @ 2022-03-07 13:36 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel 7 mars 2022 kl. 11.16 skrev Po Lu <luangruo@yahoo.com>: > See my reply to Mattias, that's not the RGBi specification I'm talking > about. Actually it is the spec that we are implementing. Let's recap the pertinent history, very roughly: 1. Emacs gets X11 support, and uses XParseColor. 2. XCMS is added to X11, so Emacs now understands rgbi:0/0.5/1 in addition to #fcb8a9 and SlimeGreen, but hardly anyone uses it. 3. Emacs is ported to Windows. For compatibility, the backend parses the rgbi: syntax just in case. 4. Emacs is ported to NextStep (etc), but the backend omits the rgbi: syntax. Nobody complains. 5. The colour spec parsing from all backends is consolidated to a single implementation and naturally it includes rgbi:. Not only is the rgbi: syntax very little used, until recently it wasn't even available on all platforms. Thus this doesn't matter much at all, and it's been fixed now because it was reported as a bug, and it was clearly my mistake to mend. I wouldn't mind dropping the support for rgbi: completely if that is all right with the maintainers. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 13:36 ` Mattias Engdegård @ 2022-03-07 13:52 ` Po Lu 0 siblings, 0 replies; 12+ messages in thread From: Po Lu @ 2022-03-07 13:52 UTC (permalink / raw) To: Mattias Engdegård; +Cc: emacs-devel Mattias Engdegård <mattiase@acm.org> writes: > 2. XCMS is added to X11, so Emacs now understands rgbi:0/0.5/1 in > addition to #fcb8a9 and SlimeGreen, but hardly anyone uses it. It happened as part of the changes in X11R5 to support device-independent color, but it was never part of the Xcms. This is all unrelated though. > 5. The colour spec parsing from all backends is consolidated to a > single implementation and naturally it includes rgbi:. And at this point, the `rgbi:' syntax in Emacs stops implementing the syntax understood by the X server. So there is no point in limiting it to what is understood by the X server again, since the new syntax doesn't break anything. Thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [External] : Re: master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs 2022-03-07 10:14 ` Po Lu 2022-03-07 13:36 ` Mattias Engdegård @ 2022-03-07 16:06 ` Drew Adams 1 sibling, 0 replies; 12+ messages in thread From: Drew Adams @ 2022-03-07 16:06 UTC (permalink / raw) To: Po Lu, Mattias Engdegård; +Cc: emacs-devel@gnu.org > If the happenstance doesn't cause trouble, there is no need to get rid > of it. As I suggested, mentioning other, similar contexts: But if you want/need to know _whether_ some data is well-formed ______ according to the standard, then you don't want to allow data that's not well-formed. Whether or not you want to support such a use case, i.e., _checking_ whether something is ONLY an rgbi code and not something that also includes whitespace, is up to you. But it's easy to provide users with both behaviors, by adding an optional arg (LAX or STRICT), depending on what you want the default behavior to be. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-03-07 16:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <164657208058.1597.5057681041192670917@vcs2.savannah.gnu.org> [not found] ` <20220306130800.EE0F9C01681@vcs2.savannah.gnu.org> 2022-03-06 13:46 ` master 6eeab90632: Don't accept whitespace or hex floats in rgbi: colour specs Po Lu 2022-03-06 13:56 ` Mattias Engdegård 2022-03-06 16:49 ` [External] : " Drew Adams 2022-03-07 0:34 ` Po Lu 2022-03-07 9:26 ` Philip Kaludercic 2022-03-07 10:16 ` Po Lu 2022-03-07 15:16 ` Philip Kaludercic 2022-03-07 9:31 ` Mattias Engdegård 2022-03-07 10:14 ` Po Lu 2022-03-07 13:36 ` Mattias Engdegård 2022-03-07 13:52 ` Po Lu 2022-03-07 16:06 ` [External] : " Drew Adams
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).