From: Alan M. <ac...@mu...> - 2006-07-24 21:24:40
|
Hi, Simon! On Mon, Jul 24, 2006 at 09:39:26AM +0100, Marshall, Simon wrote: > Hi Alan, did this get forwarded to you too? > Are you the right person to forward it to? Yes. Sorry, I've been very slow answering. The canonical address for CC Mode problems is <bug...@gn...>. > Thanks, Simon. > -----Original Message----- > From: Marshall, Simon > Sent: 22 June 2006 14:33 > To: 'Emacs Pretest Bug (ema...@gn...)' > Subject: Font-lock misfontifies foo::bar in C++ > src/emacs -Q foo.cpp > In the foo.cpp buffer, insert the text: > void foo::bar() // wrong - foo in font-lock-constant-face (otherwise ok) The face for "foo" is deliberately set to c-reference-face-name (At cc-fonts.el L667 (in (c-lang-defconst c-basic-matchers-before ....)). c-reference-face-name is defined (earlier on in the file) as c-label-face-name, which (in its turn) becomes font-lock-constant-face. So this is quite deliberate. I don't assert it's "right", for whatever value of "right". Maybe this could be described more clearly in the CC Mode manual (on page "Faces"). > { > foo ;// ok - no fontification What is this, syntactically? Is it valid C++? > foo: // ok - foo in font-lock-constant-face (also used > for labels) > foo:: ;// ok - foo not fontified (but maybe could be as a type) Is this syntactically valid C++? > foo::bar // wrong - foo in font-lock-constant-face (See above.) > The text "foo" in "foo::bar" is fontified in font-lock-constant-face, rather > than font-lock-type-face. In C++, at least, anything before a "::" is a > namespace or class name. Note that mis-fontification happens when the "b" > of "bar" is typed. > Sorry, no fix. Cc-fonts.el is too scary for my diminishing elisp skills - > and I'm wary of breaking something else anyway. :-) The fix, if such is needed, would be to redefine (defconst c-reference-face ...) at cc-fonts.el L145. > Simon. -- Alan Mackenzie (Munich, Germany). |
From: Marshall, S. <simon.marshall@Misys.com> - 2006-07-25 08:51:28
|
> > src/emacs -Q foo.cpp > > > In the foo.cpp buffer, insert the text: > > > void foo::bar() // wrong - foo in font-lock-constant-face (otherwise ok) > > The face for "foo" is deliberately set to > c-reference-face-name (At cc-fonts.el L667 (in > (c-lang-defconst c-basic-matchers-before ....)). > c-reference-face-name is defined (earlier on in the file) as > c-label-face-name, which (in its turn) becomes > font-lock-constant-face. > > So this is quite deliberate. I don't assert it's "right", > for whatever value of "right". Maybe this could be described > more clearly in the CC Mode manual (on page "Faces"). Hi Alan, why is c-reference-face-name used for foo in this context? It cannot be a reference, it is a type name. > > { > > foo ;// ok - no fontification > > What is this, syntactically? Is it valid C++? The ; was just to ensure emacs indented following lines. > > foo:: ;// ok - foo not fontified (but maybe could be as a type) > > Is this syntactically valid C++? Yes, if you ignore the ; (see above). It makes no difference to fontification. > > foo::bar // wrong - foo in font-lock-constant-face > > (See above.) > > > The text "foo" in "foo::bar" is fontified in font-lock-constant-face, > > rather than font-lock-type-face. In C++, at least, anything before a > > "::" is a namespace or class name. Note that mis-fontification happens > > when the "b" of "bar" is typed. > > :-) The fix, if such is needed, would be to redefine > (defconst c-reference-face ...) at cc-fonts.el L145. Currently, the variable c-reference-face-name is initialised to either font-lock-reference-face (an alias for font-lock-constant-face which is documented as to be used for constant and label names) or c-label-face-name (which resolves to font-lock-label-face, font-lock-constant-face or font-lock-reference-face). It is not documented, but on the face of it, c-reference-face-name suggests it is not intended to be used for type names. Am I misunderstanding the purpose of it? If you change the value of c-reference-face-name, won't it break other things that use that face name for constants and labels? Simon. |
From: Alan M. <ac...@mu...> - 2006-08-06 17:22:29
|
Hi, Simon! On Tue, Jul 25, 2006 at 09:48:45AM +0100, Marshall, Simon wrote: > > > src/emacs -Q foo.cpp > > > In the foo.cpp buffer, insert the text: > > > void foo::bar() // wrong - foo in font-lock-constant-face > (otherwise ok) [ .... ] > Hi Alan, why is c-reference-face-name used for foo in this context? It > cannot be a reference, it is a type name. I emailed Martin Stjernholm, the author of the CC Mode font locking code, and asked him about this. Here is his answer: ######################################################################### [ACM]: > Martin, can you shed any light upon why "foo" in "foo::bar" is given > c-reference-face-name? Sure. "foo" doesn't get the type face since it isn't used as a type - it's used as an identifier qualifier. Note that "foo" in a "foo::bar" expression can be a namespace name too, which isn't a type at all. So, put another way: The face is not chosen from what an identifier is defined as but rather how it is used. Some examples: Example 1: foo bar; In this statement "foo" is used to specify the type of something. Hence it gets the type face. Example 2: ~foo(); This declares a destructor for the class foo. Here "foo" doesn't specify the type of anything; it's only used in a special construct to serve as the name of the destructor function. It therefore gets font-lock-function-name-face. Example 3: foo::bar "foo" doesn't specify the type of anything here either. Its use is to tell in which scope "bar" is declared. Hence it gets a face that is different from the type face. I call this use "identifier qualifier". To make this more obvious/precise, you could perhaps introduce another face name variable, say c-qualifier-face-name, and make a suitable mapping of it onto one of the existing font-lock faces. I don't think it should be mapped to the type face though, because it would be used for namespace names too. In lack of better alternatives I chose to map it as I did. (Btw, there's not necessarily a hard distinction between types and constants in all languages. Again, it's (afaik) Pike that pushes the limits in this regard. There a type name is nothing more than a class that has been assigned to a constant.) ######################################################################### [ .... ] > Simon. -- Alan. |
From: Marshall, S. <sim...@mi...> - 2006-08-07 08:32:33
|
> > Martin, can you shed any light upon why "foo" in "foo::bar" is given > > c-reference-face-name? > > Sure. "foo" doesn't get the type face since it isn't used as > a type - it's used as an identifier qualifier. Note that > "foo" in a "foo::bar" > expression can be a namespace name too, which isn't a type at all. Alan, I already explained that it was either a type name or a namespace name. My point was that it definitely is *not* a reference (a constant or label). > Example 3: > > foo::bar > > "foo" doesn't specify the type of anything here either. Its > use is to tell in which scope "bar" is declared. Hence it > gets a face that is different from the type face. I call this > use "identifier qualifier". > > To make this more obvious/precise, you could perhaps > introduce another face name variable, say > c-qualifier-face-name, and make a suitable mapping of it onto > one of the existing font-lock faces. I don't think it should > be mapped to the type face though, because it would be used > for namespace names too. In lack of better alternatives I > chose to map it as I did. I think c-qualifier-face-name would be a good idea, but I think it odd to pick a face for its value that is definitely wrong. References are used for constants and labels. Why not pick a face that is usually right, eg, the type face? Perhaps a better solution would be to add a c++-font-lock-extra-qualifiers, akin to c++-font-lock-extra-types, that is a list of qualifier names. I would suggest that, by default, these could be fontified in font-lock-type-face, but perhaps you might like to add a font-lock-qualifier-face to allow users (or modes) to specify what they want. For C++, this variable could default to '("std") or somesuch. So, by default, "foo::bar" would not cause "foo" to be fontified in any face. If the user wanted it to be fontified, they would modify c++-font-lock-extra-types if it were a type or c++-font-lock-extra-qualifiers if it were a namespace. WDYT? I'm not on either mailing list, so please CC: me on any reply. Thanks, Simon. |
From: Alan M. <ac...@mu...> - 2006-08-16 10:43:48
|
Morning, Simon! On Mon, Aug 07, 2006 at 09:32:59AM +0100, Marshall, Simon wrote: > > > Martin, can you shed any light upon why "foo" in "foo::bar" is given > > > c-reference-face-name? > > Sure. "foo" doesn't get the type face since it isn't used as > > a type - it's used as an identifier qualifier. Note that > > "foo" in a "foo::bar" > > expression can be a namespace name too, which isn't a type at all. > Alan, I already explained that it was either a type name or a namespace > name. > My point was that it definitely is *not* a reference (a constant or label). ######################################################################### OK. Just to clarify where I think we are - In this: void foo::bar() , "foo" gets font-lock-constant-face. (i) We both agree that this face assignment is deliberate and documented: On the page "Faces" in the CC Mode manual, it says: * Name qualifiers and identifiers for scope constructs are fontified like labels. * Label identifiers get `font-lock-constant-face' if it exists, `font-lock-reference-face' otherwise. (ii) You're saying that fontifying "foo::" like a label is bad. ######################################################################### > > Example 3: > > foo::bar > > "foo" doesn't specify the type of anything here either. Its > > use is to tell in which scope "bar" is declared. Hence it > > gets a face that is different from the type face. I call this > > use "identifier qualifier". > > To make this more obvious/precise, you could perhaps > > introduce another face name variable, say > > c-qualifier-face-name, and make a suitable mapping of it onto > > one of the existing font-lock faces. I don't think it should > > be mapped to the type face though, because it would be used > > for namespace names too. In lack of better alternatives I > > chose to map it as I did. > I think c-qualifier-face-name would be a good idea, but I think it odd to > pick a face for its value that is definitely wrong. References are used for > constants and labels. Why not pick a face that is usually right, eg, the > type face? I'm at a disadvantage here, because I don't feel at all strongly about what the right face is for "foo::", as long as it's visually distinct. In fact I'd never really noticed that it was font-lock-constant-face. You're an expert on font-lock, though. Why would f-l-type-face be any better? Martin feels quite strongly, that "foo::bar" is not using foo "in a type context", and has referred to Pike (one of CC Mode's other languages), where a constant declaration is a special case of a class declaration. > Perhaps a better solution would be to add a > c++-font-lock-extra-qualifiers, akin to c++-font-lock-extra-types, > that is a list of qualifier names. I would suggest that, by default, > these could be fontified in font-lock-type-face, but perhaps you might > like to add a font-lock-qualifier-face to allow users (or modes) to > specify what they want. For C++, this variable could default to > '("std") or somesuch. > So, by default, "foo::bar" would not cause "foo" to be fontified in any > face. If the user wanted it to be fontified, they would modify > c++-font-lock-extra-types if it were a type or > c++-font-lock-extra-qualifiers if it were a namespace. > WDYT? OK; maybe. But that would definitely be a new feature for CC Mode 5.32, not something to slip in to 5.31.n in a hurry. As regards f-l-constant-face on "foo::": I don't really want to change it, since this would be a lot of work. Changing the code in cc-fonts.el and the text in the manual would be relatively quick and easy, but then I'd have to update the regression test program 000tests.el, and recalcuate all the "face files" in the test suite, diff them with the existing versions, and commit them to the CVS repository. There are 166 files.face involved. I would surely also get complaints from people who've got used to the existing fontification. So why is f-l-constant-face so jarring here? I can see that it's not particularly appropriate, but what is there about it that makes it repulsive and ghastly, that raises a calm competent hacker's hackles? (This isn't a rhetorical question.) > I'm not on either mailing list, so please CC: me on any reply. Not merely a CC, but a To: is what you get. :-) > Thanks, Simon. -- Alan. |
From: Marshall, S. <sim...@mi...> - 2006-08-16 14:34:57
|
> OK. Just to clarify where I think we are - In this: > > void foo::bar() > > , "foo" gets font-lock-constant-face. > > (i) We both agree that this face assignment is deliberate and > documented: On the page "Faces" in the CC Mode manual, it says: > * Name qualifiers and identifiers for scope constructs are > fontified like labels. > * Label identifiers get `font-lock-constant-face' if it exists, > `font-lock-reference-face' otherwise. > (ii) You're saying that fontifying "foo::" like a label is bad. > > ############################################################## > ########### > > > > Example 3: > > > > foo::bar > > > > "foo" doesn't specify the type of anything here either. Its > > > use is to tell in which scope "bar" is declared. Hence it > > > gets a face that is different from the type face. I call this > > > use "identifier qualifier". > > > > To make this more obvious/precise, you could perhaps > > > introduce another face name variable, say > > > c-qualifier-face-name, and make a suitable mapping of it onto > > > one of the existing font-lock faces. I don't think it should > > > be mapped to the type face though, because it would be used > > > for namespace names too. In lack of better alternatives I > > > chose to map it as I did. > > > I think c-qualifier-face-name would be a good idea, but I think it odd to > > pick a face for its value that is definitely wrong. References are used for > > constants and labels. Why not pick a face that is usually right, eg, the > > type face? > > I'm at a disadvantage here, because I don't feel at all strongly about > what the right face is for "foo::", as long as it's visually distinct. > In fact I'd never really noticed that it was font-lock-constant-face. > > You're an expert on font-lock, though. Why would f-l-type-face be any > better? Martin feels quite strongly, that "foo::bar" is not using foo > "in a type context", and has referred to Pike (one of CC Mode's other > languages), where a constant declaration is a special case of a class > declaration. OK, let's take this qualifier thing at face value. (In C++, "foo" in "foo::bar" is a qualifier. It is the name either of a type or a namespace.) class CLASS; // CLASS is in font-lock-type-face namespace NAMESPACE; // NAMESPACE is now in font-lock-constant-face void CORN::fubar(); // CORN is now in font-lock-constant-face (In Emacs 19/20/21, CLASS, NAMESPACE and CORN are all in font-lock-type-face.) By the qualifier argument, CORN cannot be in the face used for type names, because it is not used "in a type context". But, by the same argument, it cannot be in the face used for namespace names either, because it is not used in a namespace context. (a) Since CORN is fontified in the same face as namespace names, your qualifier argument leads me to the conclusion that the current behaviour is broken. By the qualifier argument, if you really wanted to distinguish between namespace names, type names and qualifiers, you would have to use 3 distinct faces. This leads me to my original bug report. (b) The use of font-lock-constant-face for either NAMESPACE or CORN is wrong, because that face is for constants and labels. That face is quite explicitly documented that way. Therefore, by your (a) and my (b), you need to use a new face for NAMESPACE and a new face for CORN. To return to the qualifier argument. The contexts are (1) a type name in a type context, (2) a type name in a qualifier context, (3) a namespace name in a namespace context, and (4) a namespace name in a qualifier context. Currently, you distinguish between (1) and (2,3,4). This is wrong regardless of the faces used. Personally, I do not see the value in distinguishing between each of them (I hope you don't think that the qualifier argument implies that you should!), or even between (1), (3) and (2,4). I don't buy the qualifier argument at all. There would be too many faces for one, and too little practical gain for another. To return to my original report. I originally made (1,2,3,4) be fontified as a type name mostly for ease of implementation and performance, not because I had a particular view of whether it was worth distinguishing between them. I think it would be useful to change this, so that (1,2) are distinguished from (3,4). But you shouldn't use font-lock-constant-face to fontify a namespace name. That's why I say it would be better to fontify as a type name, at least until type names and namespace names can be dealt with individually as I suggest below. (I don't see the relevance of Pike---this is just about C++.) > > Perhaps a better solution would be to add a > > c++-font-lock-extra-qualifiers, akin to c++-font-lock-extra-types, > > that is a list of qualifier names. I would suggest that, by default, > > these could be fontified in font-lock-type-face, but perhaps you might > > like to add a font-lock-qualifier-face to allow users (or modes) to > > specify what they want. For C++, this variable could default to > > '("std") or somesuch. > > > So, by default, "foo::bar" would not cause "foo" to be fontified in any > > face. If the user wanted it to be fontified, they would modify > > c++-font-lock-extra-types if it were a type or > > c++-font-lock-extra-qualifiers if it were a namespace. > > > WDYT? > > OK; maybe. But that would definitely be a new feature for CC > Mode 5.32, not something to slip in to 5.31.n in a hurry. > > As regards f-l-constant-face on "foo::": I don't really want to change > it, since this would be a lot of work. Changing the code in cc-fonts.el > and the text in the manual would be relatively quick and easy, but then > I'd have to update the regression test program 000tests.el, and > recalcuate all the "face files" in the test suite, diff them with the > existing versions, and commit them to the CVS repository. There are 166 > files.face involved. I would surely also get complaints from people > who've got used to the existing fontification. I don't recall any complaints back in the Emacs 19/20/21 days when it fontified class names and namespace names as type names. Maybe I'm being presumptive, but I'm sure those who noticed type names and namespace names now being fontified as constants/labels won't mind it going back to be as type names. > So why is f-l-constant-face so jarring here? I can see that it's not > particularly appropriate, but what is there about it that makes it > repulsive and ghastly, that raises a calm competent hacker's hackles? > (This isn't a rhetorical question.) I don't claim to be calm or competent, and I never said it was repulsive or ghastly. But, visual cues are not helpful if they are wrong, particularly when they are something as fundamental as classes and namespaces are in C++. The only thing that could raise my hackles is that I feel I'm just repeating myself. And this has been going on nearly 2 months. I don't mind resolution taking that long---nobody is paid to do this---it's the lack of progress on such a simple issue that is frustrating. But perhaps I share some of the blaim for that. Thanks, Simon. |
From: Miles B. <mil...@ne...> - 2006-08-17 01:54:42
|
"Marshall, Simon" <sim...@mi...> writes: > The only thing that could raise my hackles is that I feel I'm just repeating > myself. And this has been going on nearly 2 months. I don't mind > resolution taking that long---nobody is paid to do this---it's the lack of > progress on such a simple issue that is frustrating. Well, while I agree with you that fontifying Foo:: as a type "makes more sense" from a pedantic point of view, than fontifying as a constant, I think most people don't really care much about such details of fontification as long as it "looks OK" (mostly meaning important syntax bits are visually distinctive but not annoying). With my face setup, the current fontification looks fine, indeed I never noticed this little inconsistency. Now that I think about it, in fact, I rather _like_ that Foo:: is not fontified as a type, because in a case like: void EnvmapLight::gen_samples (const Intersect &isec, SampleRayVec &samples) const I get visually distinct faces for "void", "EnvmapLight", and "gen_samples", which helps the readability of the code a bit I think. -Miles -- I'd rather be consing. |