unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: "Felix E. Klee" <felix.klee@inka.de>
Cc: 32359@debbugs.gnu.org
Subject: bug#32359: [PATCH] Add svg-path
Date: Fri, 03 Aug 2018 16:05:08 +0300	[thread overview]
Message-ID: <83zhy3txbv.fsf@gnu.org> (raw)
In-Reply-To: <CA+m_8J10a5WE8-p+UcQ7XAjw1B_VVEjtyg320fcHxS6nf-uVow@mail.gmail.com> (felix.klee@inka.de)

> From: "Felix E. Klee" <felix.klee@inka.de>
> 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  <felix.klee@inka.de>
> +
> +	* 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.





  reply	other threads:[~2018-08-03 13:05 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03 11:07 bug#32359: [PATCH] Add svg-path Felix E. Klee
2018-08-03 13:05 ` Eli Zaretskii [this message]
2018-08-23 15:50   ` Felix E. Klee
2018-08-23 17:23     ` Eli Zaretskii
2018-08-24  9:15       ` Felix E. Klee
2019-06-23 22:15     ` Lars Ingebrigtsen
2019-06-24 14:32       ` Felix E. Klee
2019-06-24 14:39         ` Lars Ingebrigtsen
2019-07-12 14:35           ` Felix E. Klee
2019-07-12 16:02             ` Lars Ingebrigtsen
2019-07-15 13:34               ` Felix E. Klee
2019-07-15 15:12                 ` Lars Ingebrigtsen
2019-07-15 20:40                   ` Felix E. Klee
2019-07-15 20:48                     ` Noam Postavsky
2019-07-16 10:32                       ` Felix E. Klee
2019-07-15 21:13                     ` Felix E. Klee
2019-07-16  6:05                     ` Lars Ingebrigtsen
2019-07-16 10:31                       ` Felix E. Klee
2019-07-16 13:46                         ` Lars Ingebrigtsen
2019-07-24  7:49                           ` Felix E. Klee
2019-07-24 14:30                             ` Eli Zaretskii
2019-07-30 17:46                           ` Felix E. Klee
2019-07-31 20:31                             ` Lars Ingebrigtsen
2019-08-01 13:23                               ` Felix E. Klee
2019-08-02  6:40                                 ` Eli Zaretskii
2019-08-02 10:43                                   ` Felix E. Klee
2019-08-02 11:57                                     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83zhy3txbv.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=32359@debbugs.gnu.org \
    --cc=felix.klee@inka.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).