Subject: bug#32359: [PATCH] Add svg-path

From: Eli Zaretskii <[email-masked]>
Date: Fri, 03 Aug 2018 16:05:08 +0300

> From: "Felix E. Klee" <[email-masked]>
> Date: Fri, 3 Aug 2018 13:07:42 +0200
> 
> The patch adds `svg-path'. Among other things, this function makes it
> possible to add arcs to SVG images. A path is drawn using commands
> defined by the SVG standard:
> 
> https://www.w3.org/TR/SVG11/paths.html#PathData

Thanks.  This contribution can be accepted without legal paperwork,
but the next one will need a copyright assignment, so I'd encourage
you to start your paperwork now.

A few comments, mainly about the documentation parts.

> diff --git a/ChangeLog.3 b/ChangeLog.3
> index a0a4794b4e..2a9832b67b 100644
> --- a/ChangeLog.3
> +++ b/ChangeLog.3
> @@ -1,3 +1,7 @@
> +2018-08-03  Felix E. Klee  <[email-masked]>
> +
> +	* lisp/svg.el (svg-path): New function.
> +

We don't maintain a ChangeLog file; the above should be the commit log
message.

> +@defun svg-path svg commands &rest args
> +Add the outline of a shape to @var{svg}. The @var{commands} follow the
> +Scalable Vector Graphics standard. This function can be used to create
> +arcs.

This is too cryptic for the manual, and the example doesn't help
enough.  We should at least explain what "arcs" means in this context,
and in general what is this function about; also what kind of object
is SCG.

Also, please observe our standard of having 2 spaces between
sentences.

In general, GNU Coding Standards frown upon using "path" for anything
that is not PATH-style directory lists, so maybe use a different name
or explain what kind of "path" is being referenced here.  E.g., the
Web page to which you pointed does include a definition of "path" in
this context.

> +(defun svg-path (svg commands &rest args)
> +  "Add the outline of a shape to SVG. The COMMANDS follow the
> +Scalable Vector Graphics standard. This function can be used to
> +create arcs."

The first line of the doc string should be a single complete sentence,
and it should mention all of the arguments.  Also, please keep 2
spaces between sentences in the doc strings.  I also think the doc
string should say that COMMANDS are strings, or objects whose printed
representation yields valid SVG commands.

This new function should also be announced in NEWS.