Pikchr

Patch for class attributes + some rendering tweaks
Login

Patch for class attributes + some rendering tweaks

(1) By sam atman (mnemnion) on 2024-04-13 00:02:31 [link] [source]

Awhile back, I started a couple of threads about adding classes to Pikchr.

I got fairly far with it, then a little too far, the experiment described there of replacing all the style attributes with CSS was a problem, because CSS isn't lexically scoped. Turns out. Which is kind of wild, given how it's designed, but there's no changing it now.

Life intervened, as it does sometimes. Just recently I had a hankering to get back to it, so I incorporated some feedback from drh, merged the head of trunk, and generally got the patch into shape.

Here's a fiddle showing off what it's all about. Be sure to mouse over the diagram, I'm afraid it isn't mobile-friendly but it's the veritable tip of the iceberg of what's possible. Intentionally a bit silly, but consider the possibility for adding interactivity and accessibility to the sort of technical diagram where Pikchr excels. It's considerable!

I'll use the rest of this post to explain things, then reply with the start of a discussion of the changes, and reply a third time with the patch, which is pretty long.

Class keyword

The main change is to add a keyword class, which the manual could explain as "user classes" or something like that. The syntax works like this:

ALabel: class one_class box
move 
class 'two_class three_class' box

So before the object class, and after any label. These produce class attributes in the resulting SVG, which can be manipulated using CSS as shown in the fiddle above. It follows the token rule for IDs in Pikchr, of necessity for a "bare" class, and out of consistency for the shell style 'do not split my tokens' string of classes.

All build-in classes in SVG which have word separators use camelCase or kebab-case, and one of the motivators here is to be able to use aria attributes for accessibility. So all underscores are replaced with hyphens.

Rendering changes

I'll start by noting that, as far as I can tell by eye, the test diagrams are identical between this patch and the latest trunk release. Under the hood, there are a few changes.

Grouping and object classes

A [pikchr group] is now an SVG group element. Every named object gets class = "name" as one of the class attributes for its element, so what was previously just a path and a polygon now both have class = "arrow".

Any user classes on a group are inherited by everything in that group. That will enable more complex transformations, although I'm not 100% about this one, as I'll go into in the deep dive.

Presentation Attributes Instead of Style

There are no more style attributes, everything uses a presentation attribute instead. In addition, instead of fill="none", it's now fill="transparent", meaning that un-colored parts of a diagram may be filled by CSS or JavaScript. The effect is otherwise identical. The reason for this change is to make all of those choices overridable by CSS. My next post will discuss why that's good and why it shouldn't make you nervous.

Labels mean IDs

Every labeled element receives an ID consisting of that label. There was some concern about this when I brought it up, so I'll mention again that it is valid to have non-unique ID attributes, so both reusing a label, and using the same label in several diagrams embedded in the same page, will pose no problem for rendering in any SVG implementation of which I am aware. The markup isn't useful if they aren't unique, but we can expect users of Pikchr who intend to make use of the id attribute to be aware of this, and plan accordingly.

NTokens builtin

There is a three line change in response to this thread, which adds nTokens as a builtin variable, which can be printed at any point to display the number of tokens consumed so far. I'm not sure if this is useful or desirable, that thread shows that at least one person wanted this affordance. It's easily removed in any case.

Afterward

Those are the changes in the upcoming patch. I'll follow this post a bit later with some more under-the-hood discussion, soliciting some feedback on the implementation itself, its scope, and addressing some concerns (security mainly) also raised in the originating thread. And of course, the patch itself.

(2) By sam atman (mnemnion) on 2024-04-13 02:21:06 in reply to 1 [source]

I'm structuring this thread so that discussion can stay above the patch. So I'm using the first reply to anticipate concerns, and discuss some possible changes to the patch itself.

The first thing one might wonder here is: is this going to make anything worse? Will embedded Pikchr diagrams which were rendering fine start to suddenly render poorly? Does this open the door to new mischief?

It's foolish to give 100% answers about security questions, but I'm confident that the answer to the first two of these is no, not really. The biggest difference between Pikchr's current rendering and what this patch produces is that it replaces all use of the style attribute with the corresponding presentation attribute. Something which would have had style="fill:rgb(0,0,0);strokewidth:1.5" now has fill="rgb(0,0,0)" strokewidth="1.5", and so on. All fill="none" is now fill="transparent".

This makes it feasible to change those properties later, whether to style a diagram to match the rest of the page, or to add dynamics. This is simply very unlikely to happen by accident. SVG shares few elements with HTML, there may be more than <a> by an element or two but that's the main one. Pikchr doesn't emit hyperlinks, and it if did, users would probably want anchor styles to match the page: maybe, maybe not.

It could happen that a user might have general SVG styles for a page, add a Pikchr diagram, and not like how the result looks. But this is easily remedied, in different ways depending on what one wants the result to be.

A question was asked whether this would open the door to injection attacks which weren't possible before. No, not really. CSS injection attacks are just a kind of injection attack, those used to be woefully common but browsers make it much harder to set yourself up for that kind of thing by accident. Circumstances where someone could inject CSS but not JavaScript are hard to imagine, and nothing is going to save you if script injection succeeds.

How vulnerable was Pikchr output to CSS attacks before this change? Just as vulnerable, as a matter of fact. My changes are designed to make it easy and pleasant to interact with Pikchr diagrams, but a bad actor doesn't need easy and pleasant. They can open up the page, trace out the selector path to anything in the SVG, and slap an !important on the end to make it stick. Anyone who can apply user CSS maliciously can make the same changes to current-trunk Pikchr as they would be able to with branch-Pikchr. The injected CSS would be shorter, and easier to write, but it was never difficult. For completeness, I'll admit that, so far as I know, nothing CSS can do will add fill to something with the "none" attribute, so the new use of "transparent" gives some opportunities to mess with color which weren't there before.

The harder-to-answer question is whether this allows new mischief from a malicious Pikchr payload, and the answer is: right now, yes. I am able and willing to fix this in the event that this is considered for adoption.

The issue is with all elements of a group inheriting classes from that group. I thought this is the right thing to do, but I'm not actually sure, which is one reason I haven't taken the extra time to remove the vulnerability.

The current implementation stores classes in a linked list. To quote a motivating comment, "PIKCHR (and PIC) scripts do not use many variables, so it is reasonable to store them all on a linked list." Security is not my professional responsibility, but I worked until recently for a defense contractor which had a lot of security-minded people, and I do wonder what sorts of mischief could be done by script-generating a Pikchr file which creates 20,000 variables or so and then does a bunch of math with the ones at the end, or what have you. Probably not none! Rendering is absurdly fast so even this pessimal scenario is unlikely to be a real problem.

Structurally, classes should be less of a problem than even that. But this implementation copies them over to child elements, so a script which makes very long class elements (each class string is only one token) for groups, and nests them very deeply, then creates a very large number of inner elements, could cause memory exhaustion. It's not quite a zip-bomb attack, because it's O(m * n) the number of tokens and grouped elements, but it's possible that fuzzing will show problems there. By "very long" I mean that the size is stored in an unsigned int, allowing (unless lemon checks this, which it might) for 4 gigabytes and change per token on my computer. I did a case-insensitive search for "token" in lemon.c and didn't see such a limit, but this was a quick scan.

That said, given the token limit imposed after the macro bomb, Pikchr is robust against reasonably-small files. A 4GB token is a 4GB text file, after all, and it's easy to just reject very large files. But all that copying of class lists is a force multiplier, and besides, it just doesn't sit right with me. Expedient, rather than correct.

To mitigate this, I can add another level of indirection: a PXmlList would have a pointer to a PXmlClass (the linked list of class elements for a given object), another pointer to the next PXmlList, and a bKeep flag. This would be set to false for the class list associated with an object, and a copy with the flag set to true would be used for any inheriting objects. So only one copy would be responsible for freeing the list itself, and the others would simply free themselves down the list.

That would bring the memory use down to an acceptable level: classes would take only a small amount of memory more than necessary to store the text itself. But it would still be possible (remembering that an entire class list is one token, split up into individual classes) to have one thousand classes nested into groups a hundred deep, so that every inner element has a hundred thousand classes. Since Pikchr stores the entire render string in memory before writing it, this still offers some potential for making trouble, beyond what the current system would allow.

To address this, I propose adding a compile-time value MAX_CLASS_PER_ELEMENT, with, by default, a suitably generous number, perhaps ten thousand. pik_append_tag_open, which handles writing classes to zOut, would run a counter and quit after reaching this threshold. pikchrshow and other user-facing version of Pikchr could tighten this limit, perhaps quite a bit: the things which the class attribute make useful aren't really available without accompanying CSS, so the benefit isn't really there unless someone on the Fossil team decides they want to dive into doing fancy things with diagrams.

And for as long-winded as all of that is, I'm not convinced that inheriting classes on a group is a good idea in the first place. It was one of the things left over from my first foray into these changes, and I fixed it up so that it works correctly, but I'm starting to think that it won't let us do things with CSS or JavaScript that can't be done without it.

Other stuff

I've already provided a passionate argument for why adding classes to Pikchr diagrams is important, but that was a year and a half ago, so I'm briefly retread that ground. Probably the strongest argument is that people are always going to want Pikchr to do more things, and the class mechanism is a graceful way to say no. There's a fork which lets Pikchr load another SVG as an element? Just add a placeholder element and swap in the picture with one line of JavaScript. You want something in the diagram to act like a link? Can do. Animated snowmen bouncing around the screen? No problem. But you don't have to write it or maintain it.

For that matter, I'd love it if I could click on the elements in SQLite railroad diagrams which have their own sub-diagram, the ones which have links directly underneath the diagrams. You could add that with just a bit of CSS and maybe a pinch of JavaScript. Not saying you should, or should even want to, just that adding classes to Pikchr makes that kind of thing practical without adding keywords and code for links.

As to the actual syntax: I interpreted drh's suggestion of class class_name as something which would go after the object class, like

Box1: box class class_name ...

I think it's better the way I did it:

Box1: class class_name box ...

Because this is consistent with groups. Otherwise the classes would go after the closing ] and that seems too postfix to me. But I'm interested in feedback on all this. I didn't add an id keyword because I think "Any label is an ID" is easier to understand, from the Pikchr side it's three ways of doing things when we want only two.

So that's my follow-on done, I'll post the patch and we can take the conversation from there. Thanks for your consideration.

(6) By sam atman (mnemnion) on 2024-04-13 23:54:36 in reply to 2 [link] [source]

I've made some changes to eliminate the potential issues I described above.

  • Sub-elements of a group no longer inherit their classes. Maybe ironic, but I knew relatively little about the ins and outs of browsers when starting this project. CSS is designed so that it's easy to refer to all child elements along with the group element with .my-class > * {...}, or .my-class * {...} for recursive selection, so this was just noise in the output.

  • Appending multiple classes in a class list advances p->nToken by that many tokens, which I was able to verify works correctly with that handy nTokens built-in variable I added. An empty class string like class '' or class ' ' is not an error, and is counted as one token. It could be an error, seem harmless but would probably be a mistake. The point here is that it's handled safely. This was probably true before, but I've verified it.

  • Labels are now included in the class list, after the object class (group, box, etc) and before any user-supplied classes.

I did check to make sure that changing the value of nToken wouldn't cause problems elsewhere, which appears to be the case. I also spotted the 50,000 byte token length, which is generous, and satisfies my curiosity about how to deal with files containing very-large tokens. I was sure you had done so, just not how. A thousand max-length tokens is only about 48 megabytes, easily handled by a modern system.

Open question whether it's actually necessary to have each class in a list be its own token, someone trying to push the limits somehow could just make a max-length class, but they're more tokens than not, so I added them to the count.

(8.2) By sam atman (mnemnion) on 2024-04-20 16:45:49 edited from 8.1 in reply to 6 [link] [source]

As I've come to know the grammar of Pikchr better, it's clear to me that having classes prefix the object is a mistake. They should be attributes on an equal footing with any others. This raises a couple questions, however.

Most other attributes throw an error if set more than once. But other attributes don't take an arbitrary list of values, and class does, so a second use of the attributes wouldn't need to overwrite the first one, it could append to it. Then again, there's already a syntax for providing as many classes as desired, once, so maybe a second class attribute should also be an error. What Pikchr does with multiple attributes, or incompatible ones, is reasonably complex. The rule seems to be "do the right thing for this attribute", what's the right thing here?

The other question is about grouped statement lists. Classes appear in the SVG, but aren't visible in the render. They enable many useful things, I do think the feature itself is justified, and one of the benefits they bring is as an inline sort of documentation. One might imagine a circuit diagram, these have submodules and paths in the circuit, they overlap, tagging these with a class is better than comments for keeping track of what's what, especially given that they appear in the SVG and can be used accordingly.

I've come around to the opinion that Label: class circuit_3 box ... is just wrong, and classes should be able to appear wherever in the attribute list a user would like. I would favor first, though: Label: box class circuit_3 ..., because it documents everything the object belongs to, its purpose.

Would it be reasonable to have irregular syntax for groups? They get rather large, and putting classes after the group, which the new syntax would allow, is not as good as ThisGroup: class 'a_class circuit_3 circuit_5' [...].

Is that too weird? Classes could be available both as a prefix and as an attribute, for all unnamed_statements, which would be consistent, but doesn't seem ideal. "Classes are an attribute, but may also prefix a group" isn't that much of a wrinkle. They would still be allowed in a group's attributes, and whatever is decided about multiple invocations of class would apply here.

I want to make this patch consistent with the rest of the language, and could use some feedback on this, at your collective leisure. I recognize that time for Pikchr competes with your foremost professional obligations and/or free time, which is true for me as well, I just happen to have had some I want to spend on this. I feel no urgency toward review of the feature, nor an answer to these questions.

(9) By Stephan Beal (stephan) on 2024-04-20 16:55:19 in reply to 8.1 [link] [source]

Then again, there's already a syntax for providing as many classes as desired, once, so maybe a second class attribute should also be an error.

Without being familiar with those internals, that sounds like it would be the simpler approach, in that an error is consistent with existing attribute behavior.

They enable many useful things, ...

Another is JavaScript scriptability.

... classes should be able to appear wherever in the attribute list a user would like.

FWIW, agreed.

I want to make this patch consistent with the rest of the language, and could use some feedback on this ...

Slight caveat to the above feedback: my pikchr know-how is largely superficial, so please take it with a grain of salt. We have several forum-goers who are far, far better-versed in it, and hopefully one or two of them can provide more useful feedback (e.g. about grouping).

(10) By sam atman (mnemnion) on 2024-04-21 16:50:12 in reply to 9 [link] [source]

Without being familiar with those internals, that sounds like it would be the simpler approach, in that an error is consistent with existing attribute behavior.

It would be a trivial amount of extra work, because the classes are prepended to a linked list, so changing the grammar to make it possible to call an arbitrary number of times would just keep prepending.

When I say 'trivial', this is checking if the head pointer is not null and invoking an error if it is, in two places.

So that's one vote for consistency in handling duplicates. I'd add that there's no compelling advantage to defining classes twice or more. There happens to be a way to interpret it, without doing something undesirable like silently redefining a value, but how likely is it to be a syntax error? Fairly so, I would venture.

Although as I think about that, there are some interactions with macros which are worth thinking about. That might actually be a case where being able to add more classes would be expressive rather than erroneous.

... classes should be able to appear wherever in the attribute list a user would like.

FWIW, agreed.

My first shot, a year and a half ago, prepended them to objects and groups with a ~ prefix, which drh correctly pointed out isn't in the spirit of the language. But the main reason they stayed there when I switched to this syntax was to have a single rule for objects and groups. I do think that for longer groups, it reads cleaner to allow them as a prefix, with class in the attribute list as a valid option. The attribute list rule should have no syntactic exceptions, that's clear. An irregular clause which applies only to groups is at least defensible.

This question balances added complexity in the definition with some debatable questions of ergonomics and aesthetics. I'd want something like consensus on that in particular before making the changes.

Slight caveat to the above feedback: my pikchr know-how is largely superficial, so please take it with a grain of salt. We have several forum-goers who are far, far better-versed in it, and hopefully one or two of them can provide more useful feedback (e.g. about grouping).

It's a start! This would be the biggest change to the language since it was released, that deserves time for thorough consideration. I'm happy to hear from anyone with an opinion on this.

(3) By sam atman (mnemnion) on 2024-04-13 02:32:37 in reply to 1 [link] [source]

Here's the patch in question. The original had a bunch of changes because my editor removes all trailing newlines, so I wrote a little script to remove those, and any section which only had such changes. The output looks correct but if this patch doesn't compile or run, let me know and I'll provide the unexpurgated version.

License

Zero-Clause BSD license:

Changes Copyright (C) 2024-04-12 by Sam Atman <atmanistan@gmail.com>

Permission to use, copy, modify, and/or distribute this software for
any purpose with or without fee is hereby granted.

Patch

Index: pikchr.y
==================================================================
--- pikchr.y
+++ pikchr.y
@@ -145,10 +145,11 @@
 typedef struct PList PList;      /* A list of diagram objects */
 typedef struct PClass PClass;    /* Description of statements types */
 typedef double PNum;             /* Numeric value */
 typedef struct PRel PRel;        /* Absolute or percentage value */
 typedef struct PPoint PPoint;    /* A position in 2-D space */
+typedef struct PXmlClass PXmlClass; /* An XML class */
 typedef struct PVar PVar;        /* script-defined variable */
 typedef struct PBox PBox;        /* A bounding box */
 typedef struct PMacro PMacro;    /* A "define" macro */
 
 /* Compass points */
@@ -231,13 +232,22 @@
 struct PRel {
   PNum rAbs;            /* Absolute value */
   PNum rRel;            /* Value relative to current value */
 };
 
-/* A variable created by the ID = EXPR construct of the PIKCHR script 
+/* A node in a linked list of SVG classes.
 **
-** PIKCHR (and PIC) scripts do not use many varaibles, so it is reasonable
+*/
+
+struct PXmlClass {
+  const char *zClass;     /* Class identifier */
+  PXmlClass *pNext;       /* Next class in an element's list */
+};
+
+/* A variable created by the ID = EXPR construct of the PIKCHR script
+**
+** PIKCHR (and PIC) scripts do not use many variables, so it is reasonable
 ** to store them all on a linked list.
 */
 struct PVar {
   const char *zName;       /* Name of the variable */
   PNum val;                /* Value of the variable */
@@ -302,10 +312,11 @@
   const PClass *type;      /* Object type or class */
   PToken errTok;           /* Reference token for error messages */
   PPoint ptAt;             /* Reference point for the object */
   PPoint ptEnter, ptExit;  /* Entry and exit points */
   PList *pSublist;         /* Substructure for [...] objects */
+  PXmlClass *pXmlClass;    /* Optional list of assigned XML classes */
   char *zName;             /* Name assigned to this statement */
   PNum w;                  /* "width" property */
   PNum h;                  /* "height" property */
   PNum rad;                /* "radius" property */
   PNum sw;                 /* "thickness" property. (Mnemonic: "stroke width")*/
@@ -417,11 +428,13 @@
   void (*xRender)(Pik*,PObj*);            /* Render */
 };
 
 
 /* Forward declarations */
-static void pik_append(Pik*, const char*,int);
+static void pik_append(Pik*,const char*,int);
+static void pik_append_tag_open(Pik*,PObj*,const char*);
+static int pik_append_group(Pik*,PObj*);
 static void pik_append_text(Pik*,const char*,int,int);
 static void pik_append_num(Pik*,const char*,PNum);
 static void pik_append_point(Pik*,const char*,PPoint*);
 static void pik_append_x(Pik*,const char*,PNum,const char*);
 static void pik_append_y(Pik*,const char*,PNum,const char*);
@@ -488,11 +501,16 @@
 static void pik_behind(Pik*,PObj*);
 static PObj *pik_assert(Pik*,PNum,PToken*,PNum);
 static PObj *pik_position_assert(Pik*,PPoint*,PToken*,PPoint*);
 static PNum pik_dist(PPoint*,PPoint*);
 static void pik_add_macro(Pik*,PToken *pId,PToken *pCode);
-
+static void pik_set_xml_class(Pik*,PObj *pObj,PToken *pXToken);
+static void pik_set_xml_classes(Pik*,PObj *pObj,PToken *pXToken);
+static void pik_xml_class_prepend(PObj*,PXmlClass *pXNext);
+static void pik_xml_classes_append(PObj*,PXmlClass *pXList);
+static PXmlClass *pik_copy_xml_classes(PXmlClass*);
+static PXmlClass *pik_copy_xml_class(PXmlClass*);
 
 } // end %include
 
 %name pik_parser
 %token_prefix T_
@@ -592,11 +610,17 @@
 pritem ::= THICKNESS(X).   {pik_append_num(p,"",pik_value(p,X.z,X.n,0));}
 pritem ::= rvalue(X).      {pik_append_num(p,"",X);}
 pritem ::= STRING(S). {pik_append_text(p,S.z+1,S.n-2,0);}
 prsep  ::= COMMA. {pik_append(p, " ", 1);}
 
-unnamed_statement(A) ::= basetype(X) attribute_list.  
+unnamed_statement(A) ::= CLASS ID(C) unnamed_statement(X).
+                         {A = X; pik_set_xml_class(p,X,&C);}
+
+unnamed_statement(A) ::= CLASS XML_CLASSES(C) unnamed_statement(X).
+                         {A = X; pik_set_xml_classes(p,X,&C);}
+
+unnamed_statement(A) ::= basetype(X) attribute_list.
                           {A = X; pik_after_adding_attributes(p,A);}
 
 basetype(A) ::= CLASSNAME(N).            {A = pik_elem_new(p,&N,0,0); }
 basetype(A) ::= STRING(N) textposition(P).
                             {N.eCode = P; A = pik_elem_new(p,0,&N,0); }
@@ -991,10 +1015,167 @@
   { "textht",      0.5   },
   { "textwid",     0.75  },
   { "thickness",   0.015 },
 };
 
+/* XML Class Methods */
+
+/* Set a single XML class on an Object.
+**
+*/
+static void pik_set_xml_class(Pik *p, PObj *pObj, PToken *pXToken) {
+   if( pObj==0 ) return;
+   if( pXToken==0 ) return;
+   PXmlClass *pXNext = (PXmlClass *) malloc(pXToken->n+1 + sizeof(PXmlClass));
+   if( pXNext==0 ){
+     pik_error(p, 0, 0);
+     return;
+   }
+   char *z;
+   pXNext->zClass = z = (char*)&pXNext[1];
+   memcpy(z, pXToken->z, pXToken->n);
+   z[pXToken->n] = 0;
+   for( int i = 0; z[i] != 0; i++) {
+     if( z[i] == '_' ) {
+       z[i] = '-';
+     }
+   }
+   pik_xml_class_prepend(pObj, pXNext);
+}
+
+/* Set a list of XML classes on an object.
+**
+*/
+static void pik_set_xml_classes(Pik *p, PObj *pObj, PToken *pXToken) {
+  if( pObj==0 ) return;
+  if( pXToken==0 ) return;
+  int st = 1;
+  char *z = malloc(pXToken->n);
+  memcpy(z, pXToken->z, pXToken->n);
+  for( int i = 1; z[i] != 0; i++) {
+    if ( z[i] == '_' ) {
+      z[i] = '-';
+    } else if( z[i] == ' ' || z[i] == '\'' ) {
+      PXmlClass *pXNext = (PXmlClass *) malloc(i - st + 1 + sizeof(PXmlClass));
+      if( pXNext==0 ){
+        pik_error(p, 0, 0);
+        return;
+      }
+      char *cl;
+      pXNext->zClass = cl = (char*)&pXNext[1];
+      memcpy(cl, &z[st], i - st);
+      cl[i - st] = 0;
+      pik_xml_class_prepend(pObj, pXNext);
+      st = i + 1;
+    }
+  }
+  return;
+}
+
+/* Free an XML class list.
+**
+*/
+static void pik_xml_class_free(PObj *pObj){
+  PXmlClass *pXClass = pObj->pXmlClass;
+  while( pXClass ){
+    PXmlClass *pXNext = pXClass->pNext;
+    free(pXClass);
+    pXClass = pXNext;
+  }
+  pObj->pXmlClass = 0;
+}
+
+/* Copy an XML class list.
+**
+** We use this when passing group classes to the group's elements.
+**
+** It would be more frugal to maintain a second linked list of class
+** elements for inheritance, but this might be reasonable without that
+** second level if indirection.
+**
+**
+*/
+static PXmlClass *pik_copy_xml_classes(PXmlClass *pXml) {
+  PXmlClass *pXClone = pik_copy_xml_class(pXml);
+  if( pXClone==0 ) {
+     return pXClone;
+  };
+  PXmlClass *pXAcc = pXClone;
+  while( pXml->pNext ) {
+    pXAcc->pNext = pik_copy_xml_class(pXml->pNext);
+    if( pXAcc->pNext==0 ) {
+      // Free everything we've allocated so far
+      while( pXClone->pNext ){
+        PXmlClass *pXNext = pXClone->pNext;
+        free(pXClone);
+        pXClone = pXNext;
+      }
+      free(pXClone);
+      return (PXmlClass *) 0;
+    }
+    pXAcc = pXAcc->pNext;
+    pXml = pXml->pNext;
+  }
+  return pXClone;
+}
+
+/* Make a copy of one XML class.
+**
+*/
+PXmlClass *pik_copy_xml_class(PXmlClass *pXml) {
+  int len = strlen(pXml->zClass) + 1;
+  PXmlClass *pXClone = (PXmlClass *) malloc(len + sizeof(PXmlClass));
+  if( pXClone==0 ){
+      return pXClone;
+  }
+  char *z = (char*)&pXClone[1];
+  pXClone->zClass = strcpy(z, pXml->zClass);
+  pXClone->pNext = 0;
+  return pXClone;
+}
+
+/* Prepend an XML class to a PObj. The clone flag tells us if we're parsing,
+** where we're allowed to append, or rendering, where we must clone first.
+**
+** We prepend, rather than appending, because it's an O(1) operation, and the
+** order of class names in a class attribute is not significant.
+*/
+static void pik_xml_class_prepend(PObj *pObj, PXmlClass *pXPrepend) {
+   if( pXPrepend==0 ) return;
+   PXmlClass *pXNext = pXPrepend;
+   pXNext->pNext = 0;
+   if( pObj->pXmlClass==0 ){
+     pObj->pXmlClass = pXNext;
+     return;
+   }
+   // Prepend the new class to the list
+   PXmlClass *pXCurrent = pObj->pXmlClass;
+   pObj->pXmlClass = pXNext;
+   pXNext->pNext = pXCurrent;
+}
+
+/* Append a list of XML classes to the existing class list of pObj, if any.
+** We use this for class inheritance in groups.  We copy the classes to append,
+** So as to avoid a double-free.  Here we use append, because the class list will
+** grow as we enter sub-groups.  We can't avoid the copy action growing longer
+** without adding reference counting, but we may as well find the end of the
+** list we expect will be the shorter one.
+**
+*/
+static void pik_xml_classes_append(PObj *pObj, PXmlClass *pXList) {
+  if( pXList==0 ) return;
+  if( pObj->pXmlClass==0 ){
+    pObj->pXmlClass = pik_copy_xml_classes(pXList);
+    return;
+  }
+  PXmlClass *pXLast = pObj->pXmlClass;
+  while( pXLast->pNext ){
+    pXLast = pXLast->pNext;
+  }
+  pXLast->pNext = pik_copy_xml_classes(pXList);
+  return;
+}
 
 /* Methods for the "arc" class */
 static void arcInit(Pik *p, PObj *pObj){
   pObj->w = pik_value(p, "arcrad",6,0);
   pObj->h = pObj->w;
@@ -1041,16 +1222,17 @@
     pik_draw_arrowhead(p,&m,&f,pObj);
   }
   if( pObj->rarrow ){
     pik_draw_arrowhead(p,&m,&t,pObj);
   }
-  pik_append_xy(p,"<path d=\"M", f.x, f.y);
+  pik_append_tag_open(p, pObj, "path");
+  pik_append_xy(p,"d=\"M", f.x, f.y);
   pik_append_xy(p,"Q", m.x, m.y);
   pik_append_xy(p," ", t.x, t.y);
   pik_append(p,"\" ",2);
   pik_append_style(p,pObj,0);
-  pik_append(p,"\" />\n", -1);
+  pik_append(p," />\n", -1);
 
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1146,21 +1328,22 @@
   PNum h2 = 0.5*pObj->h;
   PNum rad = pObj->rad;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
     if( rad<=0.0 ){
-      pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y-h2);
+      pik_append_tag_open(p, pObj, "path");
+      pik_append_xy(p,"d=\"M", pt.x-w2,pt.y-h2);
       pik_append_xy(p,"L", pt.x+w2,pt.y-h2);
       pik_append_xy(p,"L", pt.x+w2,pt.y+h2);
       pik_append_xy(p,"L", pt.x-w2,pt.y+h2);
       pik_append(p,"Z\" ",-1);
     }else{
       /*
       **         ----       - y3
       **        /    \
       **       /      \     _ y2
       **      |        |    _ y1
       **       \      /
       **        \    /
       **         ----       _ y0
       **
@@ -1176,11 +1359,12 @@
       x2 = x3 - rad;
       y0 = pt.y - h2;
       y1 = y0 + rad;
       y3 = pt.y + h2;
       y2 = y3 - rad;
-      pik_append_xy(p,"<path d=\"M", x1, y0);
+      pik_append_tag_open(p, pObj, "path");
+      pik_append_xy(p,"d=\"M", x1, y0);
       if( x2>x1 ) pik_append_xy(p, "L", x2, y0);
       pik_append_arc(p, rad, rad, x3, y1);
       if( y2>y1 ) pik_append_xy(p, "L", x3, y2);
       pik_append_arc(p, rad, rad, x2, y3);
       if( x2>x1 ) pik_append_xy(p, "L", x1, y3);
@@ -1188,11 +1372,11 @@
       if( y2>y1 ) pik_append_xy(p, "L", x0, y1);
       pik_append_arc(p, rad, rad, x1, y0);
       pik_append(p,"Z\" ",-1);
     }
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "circle" class */
@@ -1247,15 +1431,16 @@
 
 static void circleRender(Pik *p, PObj *pObj){
   PNum r = pObj->rad;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_x(p,"<circle cx=\"", pt.x, "\"");
+    pik_append_tag_open(p, pObj, "circle");
+    pik_append_x(p,"cx=\"", pt.x, "\"");
     pik_append_y(p," cy=\"", pt.y, "\"");
     pik_append_dis(p," r=\"", r, "\" ");
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "cylinder" class */
@@ -1278,19 +1463,20 @@
     if( rad>h2 ){
       rad = h2;
     }else if( rad<0 ){
       rad = 0;
     }
-    pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y+h2-rad);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x-w2,pt.y+h2-rad);
     pik_append_xy(p,"L", pt.x-w2,pt.y-h2+rad);
     pik_append_arc(p,w2,rad,pt.x+w2,pt.y-h2+rad);
     pik_append_xy(p,"L", pt.x+w2,pt.y+h2-rad);
     pik_append_arc(p,w2,rad,pt.x-w2,pt.y+h2-rad);
     pik_append_arc(p,w2,rad,pt.x+w2,pt.y+h2-rad);
     pik_append(p,"\" ",-1);
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 static PPoint cylinderOffset(Pik *p, PObj *pObj, int cp){
   PPoint pt = cZeroPoint;
@@ -1344,15 +1530,16 @@
 }
 static void dotRender(Pik *p, PObj *pObj){
   PNum r = pObj->rad;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_x(p,"<circle cx=\"", pt.x, "\"");
+    pik_append_tag_open(p, pObj, "circle");
+    pik_append_x(p,"cx=\"", pt.x, "\"");
     pik_append_y(p," cy=\"", pt.y, "\"");
     pik_append_dis(p," r=\"", r, "\"");
     pik_append_style(p,pObj,2);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "diamond" class */
@@ -1457,16 +1644,17 @@
 static void ellipseRender(Pik *p, PObj *pObj){
   PNum w = pObj->w;
   PNum h = pObj->h;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_x(p,"<ellipse cx=\"", pt.x, "\"");
+    pik_append_tag_open(p, pObj, "ellipse");
+    pik_append_x(p,"cx=\"", pt.x, "\"");
     pik_append_y(p," cy=\"", pt.y, "\"");
     pik_append_dis(p," rx=\"", w/2.0, "\"");
     pik_append_dis(p," ry=\"", h/2.0, "\" ");
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "file" object */
@@ -1514,24 +1702,26 @@
   PPoint pt = pObj->ptAt;
   PNum mn = w2<h2 ? w2 : h2;
   if( rad>mn ) rad = mn;
   if( rad<mn*0.25 ) rad = mn*0.25;
   if( pObj->sw>=0.0 ){
-    pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y-h2);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x-w2,pt.y-h2);
     pik_append_xy(p,"L", pt.x+w2,pt.y-h2);
     pik_append_xy(p,"L", pt.x+w2,pt.y+(h2-rad));
     pik_append_xy(p,"L", pt.x+(w2-rad),pt.y+h2);
     pik_append_xy(p,"L", pt.x-w2,pt.y+h2);
     pik_append(p,"Z\" ",-1);
     pik_append_style(p,pObj,1);
-    pik_append(p,"\" />\n",-1);
-    pik_append_xy(p,"<path d=\"M", pt.x+(w2-rad), pt.y+h2);
+    pik_append(p," />\n",-1);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x+(w2-rad), pt.y+h2);
     pik_append_xy(p,"L", pt.x+(w2-rad),pt.y+(h2-rad));
     pik_append_xy(p,"L", pt.x+w2, pt.y+(h2-rad));
     pik_append(p,"\" ",-1);
     pik_append_style(p,pObj,0);
-    pik_append(p,"\" />\n",-1);
+    pik_append(p," />\n",-1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1552,21 +1742,33 @@
     return out;
   }
 #endif
   return boxOffset(p,pObj,cp);
 }
+
+/* Make a group for the object if: we have a compound, and we didn't make
+** a group with .id already.
+** Returns a sentinel flag so we know whether to close a <g>:
+**   - 0: no close (not compound)
+**   - 1: close (group made here)
+*/
+static int pik_append_group(Pik *p, PObj *pObj){
+  int needsGroup = (int) !(pObj->zName) && (pObj->larrow||pObj->rarrow||pObj->pSublist);
+  if ( needsGroup ) {
+    needsGroup = 1;
+    pik_append_tag_open(p, pObj, "g");
+    pik_append(p, ">\n", -1);
+  }
+  return needsGroup;
+}
+
 static void lineRender(Pik *p, PObj *pObj){
-  int i;
+  int enclosed = pik_append_group(p, pObj);
   if( pObj->sw>0.0 ){
-    const char *z = "<path d=\"M";
-    int n = pObj->nPath;
-    if( pObj->larrow ){
-      pik_draw_arrowhead(p,&pObj->aPath[1],&pObj->aPath[0],pObj);
-    }
-    if( pObj->rarrow ){
-      pik_draw_arrowhead(p,&pObj->aPath[n-2],&pObj->aPath[n-1],pObj);
-    }
+    pik_append_tag_open(p, pObj, "path");
+    const char *z = "d=\"M";
+    int i;
     for(i=0; i<pObj->nPath; i++){
       pik_append_xy(p,z,pObj->aPath[i].x,pObj->aPath[i].y);
       z = "L";
     }
     if( pObj->bClose ){
@@ -1574,11 +1776,22 @@
     }else{
       pObj->fill = -1.0;
     }
     pik_append(p,"\" ",-1);
     pik_append_style(p,pObj,pObj->bClose?3:0);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
+
+    int n = pObj->nPath;
+    if( pObj->larrow ){
+      pik_draw_arrowhead(p,&pObj->aPath[1],&pObj->aPath[0],pObj);
+    }
+    if( pObj->rarrow ){
+      pik_draw_arrowhead(p,&pObj->aPath[n-2],&pObj->aPath[n-1],pObj);
+    }
+  }
+  if( enclosed ){
+    pik_append(p, "</g>\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "move" class */
@@ -1652,12 +1865,12 @@
   const PPoint *a = pObj->aPath;
   PPoint m;
   PPoint an = a[n-1];
   int isMid = 0;
   int iLast = pObj->bClose ? n : n-1;
-
-  pik_append_xy(p,"<path d=\"M", a[0].x, a[0].y);
+  pik_append_tag_open(p, pObj, "path");
+  pik_append_xy(p,"d=\"M", a[0].x, a[0].y);
   m = radiusMidpoint(a[0], a[1], r, &isMid);
   pik_append_xy(p," L ",m.x,m.y);
   for(i=1; i<iLast; i++){
     an = i<n-1 ? a[i+1] : a[0];
     m = radiusMidpoint(an,a[i],r, &isMid);
@@ -1674,27 +1887,32 @@
   }else{
     pObj->fill = -1.0;
   }
   pik_append(p,"\" ",-1);
   pik_append_style(p,pObj,pObj->bClose?3:0);
-  pik_append(p,"\" />\n", -1);
+  pik_append(p," />\n", -1);
 }
 static void splineRender(Pik *p, PObj *pObj){
   if( pObj->sw>0.0 ){
     int n = pObj->nPath;
     PNum r = pObj->rad;
     if( n<3 || r<=0.0 ){
       lineRender(p,pObj);
       return;
     }
+    int enclosed = pik_append_group(p, pObj);
+    radiusPath(p,pObj,pObj->rad);
     if( pObj->larrow ){
       pik_draw_arrowhead(p,&pObj->aPath[1],&pObj->aPath[0],pObj);
     }
     if( pObj->rarrow ){
       pik_draw_arrowhead(p,&pObj->aPath[n-2],&pObj->aPath[n-1],pObj);
     }
-    radiusPath(p,pObj,pObj->rad);
+
+    if( enclosed ){
+      pik_append(p, "</g>\n", -1);
+    }
   }
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1963,14 +2181,15 @@
   }
   ddx = -w*dy;
   ddy = w*dx;
   bx = f->x + e1*dx;
   by = f->y + e1*dy;
-  pik_append_xy(p,"<polygon points=\"", t->x, t->y);
+  pik_append_tag_open(p, pObj, "polygon");
+  pik_append_xy(p,"points=\"", t->x, t->y);
   pik_append_xy(p," ",bx-ddx, by-ddy);
   pik_append_xy(p," ",bx+ddx, by+ddy);
-  pik_append_clr(p,"\" style=\"fill:",pObj->color,"\"/>\n",0);
+  pik_append_clr(p,"\" fill=",pObj->color,"\"/>\n",0);
   pik_chop(f,t,h/2);
 }
 
 /*
 ** Compute the relative offset to an edge location from the reference for a
@@ -1998,10 +2217,57 @@
   }
   memcpy(p->zOut+p->nOut, zText, n);
   p->nOut += n;
   p->zOut[p->nOut] = 0;
 }
+
+/*
+** Append a tag, and any classes, to zOut
+*/
+
+static void pik_append_tag_open(Pik *p,PObj *pObj,const char *zTag) {
+  pik_append(p, "<", -1);
+  pik_append(p, zTag, -1);
+  pik_append(p, " ", -1);
+  if( pObj==0 ) {
+    pik_append(p, "error=\"missing-pObj\" ", -1);
+    return;
+  }
+  if( pObj->zName ){
+    pik_append(p, "id=\"", -1);
+    pik_append(p, pObj->zName, -1);
+    pik_append(p, "\" ", -1);
+  }
+  pik_append(p, "class=\"", -1);
+  if( strcmp("[]", pObj->type->zName) ) {
+    pik_append(p, pObj->type->zName, -1);
+  }else{
+    pik_append(p, "group", -1);
+  }
+  if( pObj->pXmlClass==0 ) {
+    pik_append(p, "\" ", -1);
+    return;
+  }else{
+    pik_append(p, " ", -1);
+  }
+  for (PXmlClass *pXml= pObj->pXmlClass; pXml; pXml=pXml->pNext ) {
+    pik_append(p, pXml->zClass, -1);
+    if( pXml->pNext ) {
+       pik_append(p, " ", -1);
+    }
+  }
+  pik_append(p, "\" ", -1);
+
+  if( pObj->pSublist ) {
+    // copy classes
+    PList *pList = pObj->pSublist;
+    for(int i=0; i<pList->n; i++){
+      PObj *pSub = pList->a[i];
+      pik_xml_classes_append(pSub, pObj->pXmlClass);
+    }
+  }
+}
 
 /*
 ** Given a string and its length, returns true if the string begins
 ** with a construct which syntactically matches an HTML entity escape
 ** sequence (without checking for whether it's a known entity). Always
@@ -2213,19 +2479,18 @@
 */
 static void pik_append_arc(Pik *p, PNum r1, PNum r2, PNum x, PNum y){
   char buf[200];
   x = x - p->bbox.sw.x;
   y = p->bbox.ne.y - y;
      p->rScale*r1, p->rScale*r2,
      p->rScale*x, p->rScale*y);
   buf[sizeof(buf)-1] = 0;
   pik_append(p, buf, -1);
 }
 
-/* Append a style="..." text.  But, leave the quote unterminated, in case
-** the caller wants to add some more.
+/* Append presentation styles to text.
 **
 ** eFill is non-zero to fill in the background, or 0 if no fill should
 ** occur.  Non-zero values of eFill determine the "bg" flag to pik_append_clr()
 ** for cases when pObj->fill==pObj->color
 **
@@ -2233,37 +2498,36 @@
 **     2        fill and color are both foreground.  (Used by "dot" objects)
 **     3        fill and color are both background.  (Used by most other objs)
 */
 static void pik_append_style(Pik *p, PObj *pObj, int eFill){
   int clrIsBg = 0;
-  pik_append(p, " style=\"", -1);
   if( pObj->fill>=0 && eFill ){
     int fillIsBg = 1;
     if( pObj->fill==pObj->color ){
       if( eFill==2 ) fillIsBg = 0;
       if( eFill==3 ) clrIsBg = 1;
     }
-    pik_append_clr(p, "fill:", pObj->fill, ";", fillIsBg);
+    pik_append_clr(p, "fill=\"", pObj->fill, "\"", fillIsBg);
   }else{
-    pik_append(p,"fill:none;",-1);
+    pik_append(p,"fill=\"transparent\"",-1);
   }
   if( pObj->sw>=0.0 && pObj->color>=0.0 ){
     PNum sw = pObj->sw;
-    pik_append_dis(p, "stroke-width:", sw, ";");
+    pik_append_dis(p, " stroke-width=\"", sw, "\"");
     if( pObj->nPath>2 && pObj->rad<=pObj->sw ){
-      pik_append(p, "stroke-linejoin:round;", -1);
+      pik_append(p, " stroke-linejoin=\"round\"", -1);
     }
-    pik_append_clr(p, "stroke:",pObj->color,";",clrIsBg);
+    pik_append_clr(p, " stroke=\"",pObj->color,"\"",clrIsBg);
     if( pObj->dotted>0.0 ){
       PNum v = pObj->dotted;
       if( sw<2.1/p->rScale ) sw = 2.1/p->rScale;
-      pik_append_dis(p,"stroke-dasharray:",sw,"");
-      pik_append_dis(p,",",v,";");
+      pik_append_dis(p," stroke-dasharray=\"",sw,"");
+      pik_append_dis(p," ",v,"\"");
     }else if( pObj->dashed>0.0 ){
       PNum v = pObj->dashed;
-      pik_append_dis(p,"stroke-dasharray:",v,"");
-      pik_append_dis(p,",",v,";");
+      pik_append_dis(p,"stroke-dasharray\"",v,"");
+      pik_append_dis(p," ",v,"\"");
     }
   }
 }
 
 /*
@@ -2496,12 +2760,12 @@
       pik_bbox_add_xy(pBox, x+x1, orig_y+y1);
       continue;
     }
     nx += x;
     y += orig_y;
-
-    pik_append_x(p, "<text x=\"", nx, "\"");
+    pik_append_tag_open(p, pObj, "text");
+    pik_append_x(p, "x=\"", nx, "\"");
     pik_append_y(p, " y=\"", y, "\"");
     if( t->eCode & TP_RJUST ){
       pik_append(p, " text-anchor=\"end\"", -1);
     }else if( t->eCode & TP_LJUST ){
       pik_append(p, " text-anchor=\"start\"", -1);
@@ -2704,10 +2968,11 @@
 
 /* Free a single object, and its substructure */
 static void pik_elem_free(Pik *p, PObj *pObj){
   if( pObj==0 ) return;
   free(pObj->zName);
+  pik_xml_class_free(pObj);
   pik_elist_free(p, pObj->pSublist);
   free(pObj->aPath);
   free(pObj);
 }
 
@@ -3835,10 +4100,15 @@
       first = mid+1;
     }else{
       last = mid-1;
     }
   }
+  // Added in response to [this forum thread](https://pikchr.org/home/forumpost/89e73c4053d154df)
+  // It is otherwise unrelated to the functionality provided by this branch
+  if( strncmp("nTokens", z, n)==0 ) {
+    return p->nToken;
+  };
   if( pMiss ) *pMiss = 1;
   return 0.0;
 }
 static int pik_value_int(Pik *p, const char *z, int n, int *pMiss){
   return pik_round(pik_value(p,z,n,pMiss));
@@ -4443,10 +4713,14 @@
     bMoreToDo = 0;
     iThisLayer = iNextLayer;
     iNextLayer = 0x7fffffff;
     for(i=0; i<pList->n; i++){
       PObj *pObj = pList->a[i];
+      if( pObj->pSublist && pObj->iLayer==iThisLayer ){
+        pik_append_tag_open(p, pObj, "g");
+        pik_append(p, ">\n", -1);
+      };
       void (*xRender)(Pik*,PObj*);
       if( pObj->iLayer>iThisLayer ){
         if( pObj->iLayer<iNextLayer ) iNextLayer = pObj->iLayer;
         bMoreToDo = 1;
         continue; /* Defer until another round */
@@ -4458,10 +4732,13 @@
       if( xRender ){
         xRender(p, pObj);
       }
       if( pObj->pSublist ){
         pik_elist_render(p, pObj->pSublist);
+      }
+      if( pObj->pSublist && pObj->iLayer==iThisLayer ) {
+        pik_append(p, "</g>\n", -1);
       }
     }
   }while( bMoreToDo );
 
   /* If the color_debug_label value is defined, then go through
@@ -4648,10 +4925,11 @@
   { "bottom",     6,   T_BOTTOM,    0,         CP_S     },
   { "c",          1,   T_EDGEPT,    0,         CP_C     },
   { "ccw",        3,   T_CCW,       0,         0        },
   { "center",     6,   T_CENTER,    0,         CP_C     },
   { "chop",       4,   T_CHOP,      0,         0        },
+  { "class",      5,   T_CLASS,     0,         0,       },
   { "close",      5,   T_CLOSE,     0,         0        },
   { "color",      5,   T_COLOR,     0,         0        },
   { "cos",        3,   T_FUNC1,     FN_COS,    0        },
   { "cw",         2,   T_CW,        0,         0        },
   { "dashed",     6,   T_DASHED,    0,         0        },
@@ -4969,10 +5247,24 @@
         }
       }
       pToken->eType = T_ERROR;
       return 1;
     }
+    case '\'': {
+      for(i=1; (c = z[i])!=0; i++){
+        if( isalnum(z[i])  || z[i] == '_' || z[i] == ' '){
+          continue;
+        }else if( z[i] == '\''){
+          break;
+        }else{
+          pToken->eType = T_ERROR;
+          return i;
+        }
+      }
+      pToken->eType = T_XML_CLASSES;
+      return i+1;
+    }
     default: {
       c = z[0];
       if( c=='.' ){
         unsigned char c1 = z[1];
         if( islower(c1) ){

(7.4) By sam atman (mnemnion) on 2024-04-14 16:08:14 edited from 7.3 in reply to 3 [link] [source]

The updated patch with changes as described in the upper thread.

License

Zero-Clause BSD license:

Changes Copyright (C) 2024-04-12 by Sam Atman <atmanistan@gmail.com>

Permission to use, copy, modify, and/or distribute this software for
any purpose with or without fee is hereby granted.

Note

You'll see from the edit history that I've made a number of changes here, all bug fixes. If I add any further features I'll start a third patch.

Patch

This is another diff of the latest version of trunk in my repo, rather than a diff of the above diff. Seems like the right approach here.

Index: pikchr.y
==================================================================
--- pikchr.y
+++ pikchr.y
@@ -145,10 +145,11 @@
 typedef struct PList PList;      /* A list of diagram objects */
 typedef struct PClass PClass;    /* Description of statements types */
 typedef double PNum;             /* Numeric value */
 typedef struct PRel PRel;        /* Absolute or percentage value */
 typedef struct PPoint PPoint;    /* A position in 2-D space */
+typedef struct PXmlClass PXmlClass; /* An XML class */
 typedef struct PVar PVar;        /* script-defined variable */
 typedef struct PBox PBox;        /* A bounding box */
 typedef struct PMacro PMacro;    /* A "define" macro */
 
 /* Compass points */
@@ -231,13 +232,22 @@
 struct PRel {
   PNum rAbs;            /* Absolute value */
   PNum rRel;            /* Value relative to current value */
 };
 
-/* A variable created by the ID = EXPR construct of the PIKCHR script 
+/* A node in a linked list of SVG classes.
 **
-** PIKCHR (and PIC) scripts do not use many varaibles, so it is reasonable
+*/
+
+struct PXmlClass {
+  const char *zClass;     /* Class identifier */
+  PXmlClass *pNext;       /* Next class in an element's list */
+};
+
+/* A variable created by the ID = EXPR construct of the PIKCHR script
+**
+** PIKCHR (and PIC) scripts do not use many variables, so it is reasonable
 ** to store them all on a linked list.
 */
 struct PVar {
   const char *zName;       /* Name of the variable */
   PNum val;                /* Value of the variable */
@@ -302,10 +312,11 @@
   const PClass *type;      /* Object type or class */
   PToken errTok;           /* Reference token for error messages */
   PPoint ptAt;             /* Reference point for the object */
   PPoint ptEnter, ptExit;  /* Entry and exit points */
   PList *pSublist;         /* Substructure for [...] objects */
+  PXmlClass *pXmlClass;    /* Optional list of assigned XML classes */
   char *zName;             /* Name assigned to this statement */
   PNum w;                  /* "width" property */
   PNum h;                  /* "height" property */
   PNum rad;                /* "radius" property */
   PNum sw;                 /* "thickness" property. (Mnemonic: "stroke width")*/
@@ -417,11 +428,13 @@
   void (*xRender)(Pik*,PObj*);            /* Render */
 };
 
 
 /* Forward declarations */
-static void pik_append(Pik*, const char*,int);
+static void pik_append(Pik*,const char*,int);
+static void pik_append_tag_open(Pik*,PObj*,const char*);
+static int pik_append_group(Pik*,PObj*);
 static void pik_append_text(Pik*,const char*,int,int);
 static void pik_append_num(Pik*,const char*,PNum);
 static void pik_append_point(Pik*,const char*,PPoint*);
 static void pik_append_x(Pik*,const char*,PNum,const char*);
 static void pik_append_y(Pik*,const char*,PNum,const char*);
@@ -488,11 +501,13 @@
 static void pik_behind(Pik*,PObj*);
 static PObj *pik_assert(Pik*,PNum,PToken*,PNum);
 static PObj *pik_position_assert(Pik*,PPoint*,PToken*,PPoint*);
 static PNum pik_dist(PPoint*,PPoint*);
 static void pik_add_macro(Pik*,PToken *pId,PToken *pCode);
-
+static void pik_set_xml_class(Pik*,PObj *pObj,PToken *pXToken);
+static void pik_set_xml_classes(Pik*,PObj *pObj,PToken *pXToken);
+static void pik_xml_class_prepend(PObj*,PXmlClass *pXNext);
 
 } // end %include
 
 %name pik_parser
 %token_prefix T_
@@ -592,11 +607,17 @@
 pritem ::= THICKNESS(X).   {pik_append_num(p,"",pik_value(p,X.z,X.n,0));}
 pritem ::= rvalue(X).      {pik_append_num(p,"",X);}
 pritem ::= STRING(S). {pik_append_text(p,S.z+1,S.n-2,0);}
 prsep  ::= COMMA. {pik_append(p, " ", 1);}
 
-unnamed_statement(A) ::= basetype(X) attribute_list.  
+unnamed_statement(A) ::= CLASS ID(C) unnamed_statement(X).
+                         {A = X; pik_set_xml_class(p,X,&C);}
+
+unnamed_statement(A) ::= CLASS XML_CLASSES(C) unnamed_statement(X).
+                         {A = X; pik_set_xml_classes(p,X,&C);}
+
+unnamed_statement(A) ::= basetype(X) attribute_list.
                           {A = X; pik_after_adding_attributes(p,A);}
 
 basetype(A) ::= CLASSNAME(N).            {A = pik_elem_new(p,&N,0,0); }
 basetype(A) ::= STRING(N) textposition(P).
                             {N.eCode = P; A = pik_elem_new(p,0,&N,0); }
@@ -991,10 +1012,107 @@
   { "textht",      0.5   },
   { "textwid",     0.75  },
   { "thickness",   0.015 },
 };
 
+/* XML Class Methods */
+
+/* Set a single XML class on an Object.
+**
+*/
+static void pik_set_xml_class(Pik *p, PObj *pObj, PToken *pXToken) {
+   if( pObj==0 ) return;
+   if( pXToken==0 ) return;
+   PXmlClass *pXNext = (PXmlClass *) malloc(pXToken->n+1 + sizeof(PXmlClass));
+   if( pXNext==0 ){
+     pik_error(p, 0, 0);
+     return;
+   }
+   char *z;
+   pXNext->zClass = z = (char*)&pXNext[1];
+   memcpy(z, pXToken->z, pXToken->n);
+   z[pXToken->n] = 0;
+   for( int i = 0; z[i] != 0; i++) {
+     if( z[i] == '_' ) {
+       z[i] = '-';
+     }
+   }
+   pik_xml_class_prepend(pObj, pXNext);
+}
+
+/* Set a list of XML classes on an object.
+**
+*/
+static void pik_set_xml_classes(Pik *p, PObj *pObj, PToken *pXToken) {
+  if( pObj==0 ) return;
+  if( pXToken==0 ) return;
+  int st = 1;
+  char *z = malloc(pXToken->n);
+  memcpy(z, pXToken->z, pXToken->n);
+  // counter so that each class counts as a token
+  int tok_count = -1; // one token already counted
+  for( int i = 1; z[i] != 0; i++) {
+    if ( z[i] == '_' ) {
+      z[i] = '-';
+    } else if( z[i] == ' ' || z[i] == '\'' ){
+      // skip runs of whitespace
+      if( i == st){
+        st = i + 1;
+        continue;
+      }
+      PXmlClass *pXNext = (PXmlClass *) malloc(i - st + 1 + sizeof(PXmlClass));
+      if( pXNext==0 ){
+        pik_error(p, 0, 0);
+        return;
+      }
+      tok_count++;
+      char *cl;
+      pXNext->zClass = cl = (char*)&pXNext[1];
+      memcpy(cl, &z[st], i - st);
+      cl[i - st] = 0;
+      pik_xml_class_prepend(pObj, pXNext);
+      st = i + 1;
+    }
+  }
+  if (tok_count > 0){
+    p->nToken += tok_count;
+  }
+  return;
+}
+
+/* Free an XML class list.
+**
+*/
+static void pik_xml_class_free(PObj *pObj){
+  PXmlClass *pXClass = pObj->pXmlClass;
+  while( pXClass ){
+    PXmlClass *pXNext = pXClass->pNext;
+    free(pXClass);
+    pXClass = pXNext;
+  }
+  pObj->pXmlClass = 0;
+}
+
+/* Prepend an XML class to a PObj. The clone flag tells us if we're parsing,
+** where we're allowed to append, or rendering, where we must clone first.
+**
+** We prepend, rather than appending, because it's an O(1) operation, and the
+** order of class names in a class attribute is not significant.
+*/
+static void pik_xml_class_prepend(PObj *pObj, PXmlClass *pXPrepend) {
+   if( pXPrepend==0 ) return;
+   PXmlClass *pXNext = pXPrepend;
+   pXNext->pNext = 0;
+   if( pObj->pXmlClass==0 ){
+     pObj->pXmlClass = pXNext;
+     return;
+   }
+   // Prepend the new class to the list
+   PXmlClass *pXCurrent = pObj->pXmlClass;
+   pObj->pXmlClass = pXNext;
+   pXNext->pNext = pXCurrent;
+}
 
 /* Methods for the "arc" class */
 static void arcInit(Pik *p, PObj *pObj){
   pObj->w = pik_value(p, "arcrad",6,0);
   pObj->h = pObj->w;
@@ -1041,16 +1159,17 @@
     pik_draw_arrowhead(p,&m,&f,pObj);
   }
   if( pObj->rarrow ){
     pik_draw_arrowhead(p,&m,&t,pObj);
   }
-  pik_append_xy(p,"<path d=\"M", f.x, f.y);
+  pik_append_tag_open(p, pObj, "path");
+  pik_append_xy(p,"d=\"M", f.x, f.y);
   pik_append_xy(p,"Q", m.x, m.y);
   pik_append_xy(p," ", t.x, t.y);
   pik_append(p,"\" ",2);
   pik_append_style(p,pObj,0);
-  pik_append(p,"\" />\n", -1);
+  pik_append(p," />\n", -1);
 
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1146,21 +1265,22 @@
   PNum h2 = 0.5*pObj->h;
   PNum rad = pObj->rad;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
     if( rad<=0.0 ){
-      pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y-h2);
+      pik_append_tag_open(p, pObj, "path");
+      pik_append_xy(p,"d=\"M", pt.x-w2,pt.y-h2);
       pik_append_xy(p,"L", pt.x+w2,pt.y-h2);
       pik_append_xy(p,"L", pt.x+w2,pt.y+h2);
       pik_append_xy(p,"L", pt.x-w2,pt.y+h2);
       pik_append(p,"Z\" ",-1);
     }else{
       /*
       **         ----       - y3
       **        /    \
       **       /      \     _ y2
       **      |        |    _ y1
       **       \      /
       **        \    /
       **         ----       _ y0
       **
@@ -1176,11 +1296,12 @@
       x2 = x3 - rad;
       y0 = pt.y - h2;
       y1 = y0 + rad;
       y3 = pt.y + h2;
       y2 = y3 - rad;
-      pik_append_xy(p,"<path d=\"M", x1, y0);
+      pik_append_tag_open(p, pObj, "path");
+      pik_append_xy(p,"d=\"M", x1, y0);
       if( x2>x1 ) pik_append_xy(p, "L", x2, y0);
       pik_append_arc(p, rad, rad, x3, y1);
       if( y2>y1 ) pik_append_xy(p, "L", x3, y2);
       pik_append_arc(p, rad, rad, x2, y3);
       if( x2>x1 ) pik_append_xy(p, "L", x1, y3);
@@ -1188,11 +1309,11 @@
       if( y2>y1 ) pik_append_xy(p, "L", x0, y1);
       pik_append_arc(p, rad, rad, x1, y0);
       pik_append(p,"Z\" ",-1);
     }
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "circle" class */
@@ -1247,15 +1368,16 @@
 
 static void circleRender(Pik *p, PObj *pObj){
   PNum r = pObj->rad;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_x(p,"<circle cx=\"", pt.x, "\"");
+    pik_append_tag_open(p, pObj, "circle");
+    pik_append_x(p,"cx=\"", pt.x, "\"");
     pik_append_y(p," cy=\"", pt.y, "\"");
     pik_append_dis(p," r=\"", r, "\" ");
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "cylinder" class */
@@ -1278,19 +1400,20 @@
     if( rad>h2 ){
       rad = h2;
     }else if( rad<0 ){
       rad = 0;
     }
-    pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y+h2-rad);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x-w2,pt.y+h2-rad);
     pik_append_xy(p,"L", pt.x-w2,pt.y-h2+rad);
     pik_append_arc(p,w2,rad,pt.x+w2,pt.y-h2+rad);
     pik_append_xy(p,"L", pt.x+w2,pt.y+h2-rad);
     pik_append_arc(p,w2,rad,pt.x-w2,pt.y+h2-rad);
     pik_append_arc(p,w2,rad,pt.x+w2,pt.y+h2-rad);
     pik_append(p,"\" ",-1);
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 static PPoint cylinderOffset(Pik *p, PObj *pObj, int cp){
   PPoint pt = cZeroPoint;
@@ -1344,15 +1467,16 @@
 }
 static void dotRender(Pik *p, PObj *pObj){
   PNum r = pObj->rad;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_x(p,"<circle cx=\"", pt.x, "\"");
+    pik_append_tag_open(p, pObj, "circle");
+    pik_append_x(p,"cx=\"", pt.x, "\"");
     pik_append_y(p," cy=\"", pt.y, "\"");
     pik_append_dis(p," r=\"", r, "\"");
     pik_append_style(p,pObj,2);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "diamond" class */
@@ -1398,17 +1522,18 @@
 static void diamondRender(Pik *p, PObj *pObj){
   PNum w2 = 0.5*pObj->w;
   PNum h2 = 0.5*pObj->h;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x-w2,pt.y);
     pik_append_xy(p,"L", pt.x,pt.y-h2);
     pik_append_xy(p,"L", pt.x+w2,pt.y);
     pik_append_xy(p,"L", pt.x,pt.y+h2);
     pik_append(p,"Z\" ",-1);
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1457,16 +1582,17 @@
 static void ellipseRender(Pik *p, PObj *pObj){
   PNum w = pObj->w;
   PNum h = pObj->h;
   PPoint pt = pObj->ptAt;
   if( pObj->sw>=0.0 ){
-    pik_append_x(p,"<ellipse cx=\"", pt.x, "\"");
+    pik_append_tag_open(p, pObj, "ellipse");
+    pik_append_x(p,"cx=\"", pt.x, "\"");
     pik_append_y(p," cy=\"", pt.y, "\"");
     pik_append_dis(p," rx=\"", w/2.0, "\"");
     pik_append_dis(p," ry=\"", h/2.0, "\" ");
     pik_append_style(p,pObj,3);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "file" object */
@@ -1514,24 +1640,26 @@
   PPoint pt = pObj->ptAt;
   PNum mn = w2<h2 ? w2 : h2;
   if( rad>mn ) rad = mn;
   if( rad<mn*0.25 ) rad = mn*0.25;
   if( pObj->sw>=0.0 ){
-    pik_append_xy(p,"<path d=\"M", pt.x-w2,pt.y-h2);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x-w2,pt.y-h2);
     pik_append_xy(p,"L", pt.x+w2,pt.y-h2);
     pik_append_xy(p,"L", pt.x+w2,pt.y+(h2-rad));
     pik_append_xy(p,"L", pt.x+(w2-rad),pt.y+h2);
     pik_append_xy(p,"L", pt.x-w2,pt.y+h2);
     pik_append(p,"Z\" ",-1);
     pik_append_style(p,pObj,1);
-    pik_append(p,"\" />\n",-1);
-    pik_append_xy(p,"<path d=\"M", pt.x+(w2-rad), pt.y+h2);
+    pik_append(p," />\n",-1);
+    pik_append_tag_open(p, pObj, "path");
+    pik_append_xy(p,"d=\"M", pt.x+(w2-rad), pt.y+h2);
     pik_append_xy(p,"L", pt.x+(w2-rad),pt.y+(h2-rad));
     pik_append_xy(p,"L", pt.x+w2, pt.y+(h2-rad));
     pik_append(p,"\" ",-1);
     pik_append_style(p,pObj,0);
-    pik_append(p,"\" />\n",-1);
+    pik_append(p," />\n",-1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1552,21 +1680,33 @@
     return out;
   }
 #endif
   return boxOffset(p,pObj,cp);
 }
+
+/* Make a group for the object if: we have a compound, and we didn't make
+** a group with .id already.
+** Returns a sentinel flag so we know whether to close a <g>:
+**   - 0: no close (not compound)
+**   - 1: close (group made here)
+*/
+static int pik_append_group(Pik *p, PObj *pObj){
+  int needsGroup = (int) !(pObj->zName) && (pObj->larrow||pObj->rarrow||pObj->pSublist);
+  if ( needsGroup ) {
+    needsGroup = 1;
+    pik_append_tag_open(p, pObj, "g");
+    pik_append(p, ">\n", -1);
+  }
+  return needsGroup;
+}
+
 static void lineRender(Pik *p, PObj *pObj){
-  int i;
+  int enclosed = pik_append_group(p, pObj);
   if( pObj->sw>0.0 ){
-    const char *z = "<path d=\"M";
-    int n = pObj->nPath;
-    if( pObj->larrow ){
-      pik_draw_arrowhead(p,&pObj->aPath[1],&pObj->aPath[0],pObj);
-    }
-    if( pObj->rarrow ){
-      pik_draw_arrowhead(p,&pObj->aPath[n-2],&pObj->aPath[n-1],pObj);
-    }
+    pik_append_tag_open(p, pObj, "path");
+    const char *z = "d=\"M";
+    int i;
     for(i=0; i<pObj->nPath; i++){
       pik_append_xy(p,z,pObj->aPath[i].x,pObj->aPath[i].y);
       z = "L";
     }
     if( pObj->bClose ){
@@ -1574,11 +1714,22 @@
     }else{
       pObj->fill = -1.0;
     }
     pik_append(p,"\" ",-1);
     pik_append_style(p,pObj,pObj->bClose?3:0);
-    pik_append(p,"\" />\n", -1);
+    pik_append(p," />\n", -1);
+
+    int n = pObj->nPath;
+    if( pObj->larrow ){
+      pik_draw_arrowhead(p,&pObj->aPath[1],&pObj->aPath[0],pObj);
+    }
+    if( pObj->rarrow ){
+      pik_draw_arrowhead(p,&pObj->aPath[n-2],&pObj->aPath[n-1],pObj);
+    }
+  }
+  if( enclosed ){
+    pik_append(p, "</g>\n", -1);
   }
   pik_append_txt(p, pObj, 0);
 }
 
 /* Methods for the "move" class */
@@ -1652,12 +1803,12 @@
   const PPoint *a = pObj->aPath;
   PPoint m;
   PPoint an = a[n-1];
   int isMid = 0;
   int iLast = pObj->bClose ? n : n-1;
-
-  pik_append_xy(p,"<path d=\"M", a[0].x, a[0].y);
+  pik_append_tag_open(p, pObj, "path");
+  pik_append_xy(p,"d=\"M", a[0].x, a[0].y);
   m = radiusMidpoint(a[0], a[1], r, &isMid);
   pik_append_xy(p," L ",m.x,m.y);
   for(i=1; i<iLast; i++){
     an = i<n-1 ? a[i+1] : a[0];
     m = radiusMidpoint(an,a[i],r, &isMid);
@@ -1674,27 +1825,32 @@
   }else{
     pObj->fill = -1.0;
   }
   pik_append(p,"\" ",-1);
   pik_append_style(p,pObj,pObj->bClose?3:0);
-  pik_append(p,"\" />\n", -1);
+  pik_append(p," />\n", -1);
 }
 static void splineRender(Pik *p, PObj *pObj){
   if( pObj->sw>0.0 ){
     int n = pObj->nPath;
     PNum r = pObj->rad;
     if( n<3 || r<=0.0 ){
       lineRender(p,pObj);
       return;
     }
+    int enclosed = pik_append_group(p, pObj);
+    radiusPath(p,pObj,pObj->rad);
     if( pObj->larrow ){
       pik_draw_arrowhead(p,&pObj->aPath[1],&pObj->aPath[0],pObj);
     }
     if( pObj->rarrow ){
       pik_draw_arrowhead(p,&pObj->aPath[n-2],&pObj->aPath[n-1],pObj);
     }
-    radiusPath(p,pObj,pObj->rad);
+
+    if( enclosed ){
+      pik_append(p, "</g>\n", -1);
+    }
   }
   pik_append_txt(p, pObj, 0);
 }
 
 
@@ -1963,14 +2119,15 @@
   }
   ddx = -w*dy;
   ddy = w*dx;
   bx = f->x + e1*dx;
   by = f->y + e1*dy;
-  pik_append_xy(p,"<polygon points=\"", t->x, t->y);
+  pik_append_tag_open(p, pObj, "polygon");
+  pik_append_xy(p,"points=\"", t->x, t->y);
   pik_append_xy(p," ",bx-ddx, by-ddy);
   pik_append_xy(p," ",bx+ddx, by+ddy);
-  pik_append_clr(p,"\" style=\"fill:",pObj->color,"\"/>\n",0);
+  pik_append_clr(p,"\" fill=\"",pObj->color,"\" />\n",0);
   pik_chop(f,t,h/2);
 }
 
 /*
 ** Compute the relative offset to an edge location from the reference for a
@@ -1998,10 +2155,47 @@
   }
   memcpy(p->zOut+p->nOut, zText, n);
   p->nOut += n;
   p->zOut[p->nOut] = 0;
 }
+
+/*
+** Append a tag, and any classes, to zOut
+*/
+
+static void pik_append_tag_open(Pik *p,PObj *pObj,const char *zTag) {
+  pik_append(p, "<", 1);
+  pik_append(p, zTag, -1);
+  pik_append(p, " ", 1);
+  if( pObj==0 ) {
+    pik_append(p, "error=\"missing-pObj\" ", -1);
+    return;
+  }
+  pik_append(p, "class=\"", -1);
+  if( strcmp("[]", pObj->type->zName) ) {
+    pik_append(p, pObj->type->zName, -1);
+  }else{
+    pik_append(p, "group", -1);
+  }
+  if( pObj->zName ){
+    pik_append(p, " ", 1);
+    pik_append(p, pObj->zName, -1);
+  }
+  if( pObj->pXmlClass==0 ) {
+    pik_append(p, "\" ", 2);
+    return;
+  }else{
+    pik_append(p, " ", 1);
+  }
+  for (PXmlClass *pXml= pObj->pXmlClass; pXml; pXml=pXml->pNext ) {
+    pik_append(p, pXml->zClass, -1);
+    if( pXml->pNext ) {
+       pik_append(p, " ", 1);
+    }
+  }
+  pik_append(p, "\" ", 2);
+}
 
 /*
 ** Given a string and its length, returns true if the string begins
 ** with a construct which syntactically matches an HTML entity escape
 ** sequence (without checking for whether it's a known entity). Always
@@ -2213,19 +2407,18 @@
 */
 static void pik_append_arc(Pik *p, PNum r1, PNum r2, PNum x, PNum y){
   char buf[200];
   x = x - p->bbox.sw.x;
   y = p->bbox.ne.y - y;
      p->rScale*r1, p->rScale*r2,
      p->rScale*x, p->rScale*y);
   buf[sizeof(buf)-1] = 0;
   pik_append(p, buf, -1);
 }
 
-/* Append a style="..." text.  But, leave the quote unterminated, in case
-** the caller wants to add some more.
+/* Append presentation styles to text.
 **
 ** eFill is non-zero to fill in the background, or 0 if no fill should
 ** occur.  Non-zero values of eFill determine the "bg" flag to pik_append_clr()
 ** for cases when pObj->fill==pObj->color
 **
@@ -2233,37 +2426,36 @@
 **     2        fill and color are both foreground.  (Used by "dot" objects)
 **     3        fill and color are both background.  (Used by most other objs)
 */
 static void pik_append_style(Pik *p, PObj *pObj, int eFill){
   int clrIsBg = 0;
-  pik_append(p, " style=\"", -1);
   if( pObj->fill>=0 && eFill ){
     int fillIsBg = 1;
     if( pObj->fill==pObj->color ){
       if( eFill==2 ) fillIsBg = 0;
       if( eFill==3 ) clrIsBg = 1;
     }
-    pik_append_clr(p, "fill:", pObj->fill, ";", fillIsBg);
+    pik_append_clr(p, "fill=\"", pObj->fill, "\"", fillIsBg);
   }else{
-    pik_append(p,"fill:none;",-1);
+    pik_append(p,"fill=\"transparent\"",-1);
   }
   if( pObj->sw>=0.0 && pObj->color>=0.0 ){
     PNum sw = pObj->sw;
-    pik_append_dis(p, "stroke-width:", sw, ";");
+    pik_append_dis(p, " stroke-width=\"", sw, "\"");
     if( pObj->nPath>2 && pObj->rad<=pObj->sw ){
-      pik_append(p, "stroke-linejoin:round;", -1);
+      pik_append(p, " stroke-linejoin=\"round\"", -1);
     }
-    pik_append_clr(p, "stroke:",pObj->color,";",clrIsBg);
+    pik_append_clr(p, " stroke=\"",pObj->color,"\"",clrIsBg);
     if( pObj->dotted>0.0 ){
       PNum v = pObj->dotted;
       if( sw<2.1/p->rScale ) sw = 2.1/p->rScale;
-      pik_append_dis(p,"stroke-dasharray:",sw,"");
-      pik_append_dis(p,",",v,";");
+      pik_append_dis(p," stroke-dasharray=\"",sw,"");
+      pik_append_dis(p," ",v,"\"");
     }else if( pObj->dashed>0.0 ){
       PNum v = pObj->dashed;
-      pik_append_dis(p,"stroke-dasharray:",v,"");
-      pik_append_dis(p,",",v,";");
+      pik_append_dis(p,"stroke-dasharray\"",v,"");
+      pik_append_dis(p," ",v,"\"");
     }
   }
 }
 
 /*
@@ -2496,12 +2688,12 @@
       pik_bbox_add_xy(pBox, x+x1, orig_y+y1);
       continue;
     }
     nx += x;
     y += orig_y;
-
-    pik_append_x(p, "<text x=\"", nx, "\"");
+    pik_append_tag_open(p, pObj, "text");
+    pik_append_x(p, "x=\"", nx, "\"");
     pik_append_y(p, " y=\"", y, "\"");
     if( t->eCode & TP_RJUST ){
       pik_append(p, " text-anchor=\"end\"", -1);
     }else if( t->eCode & TP_LJUST ){
       pik_append(p, " text-anchor=\"start\"", -1);
@@ -2704,10 +2896,11 @@
 
 /* Free a single object, and its substructure */
 static void pik_elem_free(Pik *p, PObj *pObj){
   if( pObj==0 ) return;
   free(pObj->zName);
+  pik_xml_class_free(pObj);
   pik_elist_free(p, pObj->pSublist);
   free(pObj->aPath);
   free(pObj);
 }
 
@@ -3835,10 +4028,15 @@
       first = mid+1;
     }else{
       last = mid-1;
     }
   }
+  // Added in response to [this forum thread](https://pikchr.org/home/forumpost/89e73c4053d154df)
+  // It is otherwise unrelated to the functionality provided by this branch
+  if( strncmp("nTokens", z, n)==0 ) {
+    return p->nToken;
+  };
   if( pMiss ) *pMiss = 1;
   return 0.0;
 }
 static int pik_value_int(Pik *p, const char *z, int n, int *pMiss){
   return pik_round(pik_value(p,z,n,pMiss));
@@ -4443,10 +4641,14 @@
     bMoreToDo = 0;
     iThisLayer = iNextLayer;
     iNextLayer = 0x7fffffff;
     for(i=0; i<pList->n; i++){
       PObj *pObj = pList->a[i];
+      if( pObj->pSublist && pObj->iLayer==iThisLayer ){
+        pik_append_tag_open(p, pObj, "g");
+        pik_append(p, ">\n", -1);
+      };
       void (*xRender)(Pik*,PObj*);
       if( pObj->iLayer>iThisLayer ){
         if( pObj->iLayer<iNextLayer ) iNextLayer = pObj->iLayer;
         bMoreToDo = 1;
         continue; /* Defer until another round */
@@ -4458,10 +4660,13 @@
       if( xRender ){
         xRender(p, pObj);
       }
       if( pObj->pSublist ){
         pik_elist_render(p, pObj->pSublist);
+      }
+      if( pObj->pSublist && pObj->iLayer==iThisLayer ) {
+        pik_append(p, "</g>\n", -1);
       }
     }
   }while( bMoreToDo );
 
   /* If the color_debug_label value is defined, then go through
@@ -4648,10 +4853,11 @@
   { "bottom",     6,   T_BOTTOM,    0,         CP_S     },
   { "c",          1,   T_EDGEPT,    0,         CP_C     },
   { "ccw",        3,   T_CCW,       0,         0        },
   { "center",     6,   T_CENTER,    0,         CP_C     },
   { "chop",       4,   T_CHOP,      0,         0        },
+  { "class",      5,   T_CLASS,     0,         0,       },
   { "close",      5,   T_CLOSE,     0,         0        },
   { "color",      5,   T_COLOR,     0,         0        },
   { "cos",        3,   T_FUNC1,     FN_COS,    0        },
   { "cw",         2,   T_CW,        0,         0        },
   { "dashed",     6,   T_DASHED,    0,         0        },
@@ -4969,10 +5175,24 @@
         }
       }
       pToken->eType = T_ERROR;
       return 1;
     }
+    case '\'': {
+      for(i=1; (c = z[i])!=0; i++){
+        if( isalnum(z[i])  || z[i] == '_' || z[i] == ' '){
+          continue;
+        }else if( z[i] == '\''){
+          break;
+        }else{
+          pToken->eType = T_ERROR;
+          return i+1;
+        }
+      }
+      pToken->eType = T_XML_CLASSES;
+      return i+1;
+    }
     default: {
       c = z[0];
       if( c=='.' ){
         unsigned char c1 = z[1];
         if( islower(c1) ){

(4) By Stephan Beal (stephan) on 2024-04-13 10:12:53 in reply to 1 [link] [source]

Class keyword ... Grouping and object classes

FWIW, i consider these to be a significant under-the-hood improvements, especially the addition of CSS classes.

so I'll mention again that it is valid to have non-unique ID attributes...

It's valid, yes, but not useful, as only one of those IDs will be selected by a browser for CSS selectors and browsing to the ID using #fragment links. i recommend against adding ID attributes, primarily because there's no(?) real justification for addressing these using #fragment links, but browsers will automatically make them addressable that way if an ID is set.

(5) By sam atman (mnemnion) on 2024-04-13 23:21:09 in reply to 4 [link] [source]

This prompted me to do some reading up on what ids are uniquely good for, versus classes, and I've come around to thinking these should be additional classes in the generated SVG, rather than IDs.

There aren't many cases where you need an ID rather than a class, and they don't apply here. I wanted a clear separation of labels from object and user classes, but that isn't actually necessary. The clinch is that browser engines don't handle duplicate id fields in identical fashion, so the combination of duplicate IDs and CSS rules referencing them could result in different renderings in different browsers, which we very much do not want. If someone wants to reuse a label, and also wants to refer uniquely to the labeled object, they'll have two choices: change a label, or attach a class. Whatever they decide it will render consistently, which is the critical part.

Thanks for your encouragement and feedback. I've made a few changes since I posted the patch, and I'll add that as well.