bug#32359: [PATCH] Add svg-path

[Eli Zaretskii <eliz@gnu.org>]

> From: "Felix E. Klee" <felix.klee@inka.de>
> Date: Thu, 23 Aug 2018 17:50:08 +0200
> Cc: 32359@debbugs.gnu.org
> 
> thanks for the feedback!

Thanks for working on improving Emacs.

> > We don't maintain a ChangeLog file; the above should be the commit
> > log message.
> 
> Well, the Emacs info page on committing patches states: “Write the
> change log entries for your changes. […]”

The relevant place to look nowadays is CONTRIBUTE in the top-level
directory of the Emacs tree.

> Furthermore, it links to a page explaining `ChangeLog' files in
> particular. Is that documentation outdated?

It says "change logs", not ChangeLog.  However, since it confused you,
I have now replaced that with "commit log".  The reference to the GNU
Coding Standards is still relevant, I think, because we still use the
ChangeLog style in our commit log messages.

> `ChangeLog.3' contains entries for previous additions to `svg.el', the
> latest one as recent as September 2017.

That file is auto-generated nowadays from Git log.

> Anyhow, if you want a commit log instead of a `ChangeLog' entry, how
> should I submit the commit?

The best is in "git format-patch" format, which will include your
commit log message.  If that's hard or complicated for you, a normal
patch will do with the commit log as preamble, i.e. not in Diff
format.

> >> +@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,
> 
> It’s basically in line with the description for the other `svg' functions.

Maybe so, but there's no reason to continue that practice when we add
new functions.  It is also okay to fix documentation of those other
functions, but of course it doesn't have to be part of this particular
changeset.

> > and the example doesn't help enough.
> 
> For the intended audience, i.e. those knowing how to author SVG
> documents, it should be clear.

Okay, but the manual exists not only for those already "in the know".
It should also cater to those who are studying the subject with the
purpose of writing some code for the first time in this domain.

> What I could do is add aliases for the path commands:
> 
>   * `moveto-relative' → `m'
> 
>   * `moveto-absolute' → `M'
> 
>   * etc.

I think it would be better simply to explain those in plain English,
not by adding code.

> The example would then turn into:
> 
>     (svg-path svg '((moveto-absolute 100 300)
>                     (arc-absolute 300 300 0 0 0 300 100)
>                     (closepath-absolute))
>               :stroke-color "blue" :fill-color "yellow")
> 
> Still users would need to know the command parameters which are
> detailed in every comprehensive documentation about SVG.

Exactly.  So it's slightly better (less cryptic), but still not clear
enough, because the meaning of moveto-relative and moveto-absolute is
not entirely obvious, only their general idea is evident ("relative"
vs "absolute").

I understand that it isn't reasonable to have the entire SVG docs
there, but we should explain a bit more before pointing to the
official SVG docs for further details.  I trust you that you will know
where to draw the line.

> > We should at least explain what "arcs" means in this context,
> 
> “arcs” in the context of SVG, i.e. vector graphics refers to curved
> paths:
> 
> https://www.merriam-webster.com/dictionary/arc

I didn't mean explain to me, I meant explain in the manual.

> > and in general what is this function about; also what kind of object
> > is SCG.
> 
> Quote from the documentation in the elisp info page for `svg.el': “SVG
> (Scalable Vector Graphics) is an XML format for specifying images.”

We are mis-communicating.  I didn't mean SVG the acronym, I meant SVG
the argument of the function.  The doc strings says

  Add the outline of a shape to SVG.

It is quite clear that "SVG" here doesn't stand for Scalable Vector
Graphics, it stands for some object to which the function will add a
shape.  I'm asking to say a word or two about what kind of object is
that, from the Lisp program POV.  is it a string? a symbol? a list? a
buffer? something else?

> > 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.
> 
> Renaming `path' would be super confusing to those familiar with vector
> graphics. If `path' needs to be avoided, then I’d rather not add
> `svg-path'.

There's the second alternative: explain in a few words what kind of
"path" is meant here.  Then you can use the word freely.

> BTW I got in contact with Lars, the original author of `svg.el', and
> he’s OK with the changes I proposed, including some additional
> functions.

That's fine, but the way our patch review process works, the comments
are additive ;-)

Thanks.