Re: new package: sketch-mode

[dalanicolai]

Ah... once more the link to the package

https://github.com/dalanicolai/sketch-mode

On Wed, 15 Sept 2021 at 17:42, dalanicolai <dalanicolai@gmail.com> wrote:

Hi Stefan (et al.)

The paperwork is ready. Do I need to send the contract somewhere?

Also thank you for the patch and the tip. I have applied the patch, and I am checking things regularly by byte-compiling the file.

I am still getting some

`... function` is for interactive use only

warnings, but I am not sure how to best deal with that, so I better ask here (they all refer to transient suffixes)

I worked a little more on the package, so that it has become again a little nicer. Of course, there are still many possibilities to improve it,

but it is a nice start (e.g. would be nice to merge some canvas-mode features in it, i.e. freehand drawing and mouse

selection/moving objects, then it would become a very nice(r) alternative to

https://marketplace.visualstudio.com/items?itemName=hediet.vscode-drawio ; just noticed that package very recently).

Daniel

On Tue, 31 Aug 2021 at 00:45, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Cool! Thanks. Yeah, for sure I am happy with that. So I will fill in the
> paperform request.

Let me know when it's done.

> It might still take some work/time to prepare the package for ELPA though.

In the mean time, here's a patch which addresses some of the warnings
emitted by the byte-compiler.  I strongly recommend to byte-compile your
code on a regular basis and pay attention to the warnings.


        Stefan


diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000000..aa3ed33912
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,3 @@
+*.elc
+/sketch-mode-autoloads.el
+/sketch-mode-pkg.el
diff --git a/sketch-mode.el b/sketch-mode.el
index bedd344eaa..d59672f63f 100644
--- a/sketch-mode.el
+++ b/sketch-mode.el
@@ -1,10 +1,10 @@
 ;;; sketch-mode.el --- Quickly create svg sketches using keyboard and mouse -*- lexical-binding: t; -*-

-;; Copyright (C) 2021 Daniel Nicolai
-
+;; Copyright (C) 2021  Free Software Foundation, Inc.

 ;; Author: D.L. Nicolai <dalanicolai@gmail.com>
 ;; Created: 17 Jul 2021
+;; Version: 0

 ;; Keywords: multimedia
 ;; URL: https://github.com/dalanicolai/sketch-mode
@@ -79,7 +79,7 @@
   "Default size for sketch canvas.
 Cons cell with car and cdr both integers, respectively
 representing the image width and image height
-(default: '(800 . 600))."
+default: (800 . 600)."
   :type '(cons integer integer))

 (defcustom sketch-show-grid t
@@ -134,7 +134,7 @@ STOPS is a list of percentage/color pairs."
   (svg--def
    svg
    (apply
-    'dom-node
+    #'dom-node
     'marker
     `((id . ,id)
       (viewBox . "0 0 10 10")
@@ -174,8 +174,8 @@ transient."
            (expt (- (cdr end-coords) (cdr start-coords)) 2))))

 (defun sketch--rectangle-coords (start-coords end-coords)
-  (let ((base-coords (cons (apply 'min (list (car start-coords) (car end-coords)))
-                           (apply 'min (list (cdr start-coords) (cdr end-coords))))))
+  (let ((base-coords (cons (apply #'min (list (car start-coords) (car end-coords)))
+                           (apply #'min (list (cdr start-coords) (cdr end-coords))))))
   (list (car base-coords)
         (cdr base-coords)
         (abs (- (car end-coords) (car start-coords)))
@@ -187,12 +187,13 @@ transient."
         (abs (/ (- (car end-coords) (car start-coords)) 2))
         (abs (/ (- (cdr end-coords) (cdr start-coords)) 2))))

+(defvar svg) ;FIXME: Use a longer name with `sketch-' prefix for dynbound vars!
+(defvar svg-canvas)
+(defvar svg-grid)
+(defvar svg-sketch)
+
 (defun sketch--create-canvas (width height &optional grid-param)
   "Create canvas for drawing svg using the mouse."
-  (defvar svg)
-  (defvar svg-canvas)
-  (defvar svg-grid)
-  (defvar svg-sketch)
     (insert-image
      (let ((width width)
            (height height))
@@ -240,6 +241,8 @@ values"
       (let ((width (if arg (car sketch-default-image-size) (read-number "Enter width: ") ))
             (height (if arg 600 (read-number "Enter height: "))))
         (switch-to-buffer (get-buffer-create "*sketch*"))
+        ;; FIXME: `defvar' can't be meaningfully inside a function like that.
+        ;; FIXME: Use a `sketch-' prefix for all dynbound vars.
         (defvar-local sketch-elements nil)
         (defvar-local grid-param 25)
         (setq grid-param (if arg 25 (read-number "Enter grid parameter (enter 0 for no grid): ")))
@@ -304,7 +307,7 @@ values"
   ((fallback    :initarg :fallback    :initform nil)
    (default     :initarg :default     :initform nil)))

-(cl-defmethod transient-infix-read ((obj sketch-variable:colors))
+(cl-defmethod transient-infix-read ((_obj sketch-variable:colors))
   (read-color "Select color: "))

 (cl-defmethod transient-infix-value ((obj sketch-variable:colors))
@@ -314,19 +317,24 @@ values"
       (when default
         (concat (oref obj argument) (substring-no-properties default))))))

+;; We always call the autoloaded `color-name-to-rgb' before calling this
+;; function, so we know it's available even tho the compiler doesn't.
+(declare-function color-rgb-to-hex "color"
+                  (red green blue &optional digits-per-component))
+
 (cl-defmethod transient-format-value ((obj sketch-variable:colors))
   (let ((value (oref obj value))
         (default  (oref obj default)))
     (if value
         (format "%s (%s)"
                 (propertize value 'face (cons 'foreground-color value))
-                (propertize (apply 'color-rgb-to-hex (color-name-to-rgb value))
+                (propertize (apply #'color-rgb-to-hex (color-name-to-rgb value))
                             'face 'transient-inactive-argument))
       (if (string= default "none")
           (propertize "none" 'face 'transient-inactive-argument)
         (format "%s (%s)"
                 (propertize default 'face (cons 'foreground-color default))
-                (propertize (apply 'color-rgb-to-hex (color-name-to-rgb default))
+                (propertize (apply #'color-rgb-to-hex (color-name-to-rgb default))
                             'face 'transient-inactive-argument))))))

   ;; (let* ((args (when transient-current-prefix (transient-args 'sketch-transient)))
@@ -474,7 +482,7 @@ values"
 (defun sketch-create-label ()
   (interactive)
   (let* ((alphabet "abcdefghijklmnopqrstuvwxyz")
-         (labels-list (mapcar 'string (concat alphabet (upcase alphabet))))
+         (labels-list (mapcar #'string (concat alphabet (upcase alphabet))))
          (labels (sketch-labels-list)))
     (while (member (car labels-list) labels)
       (setq labels-list (cdr labels-list)))
@@ -489,7 +497,8 @@ values"
   (dolist (coord args node)
     (cl-decf (alist-get coord (cadr node)) amount)))

-(defun svg-translate (dx dy)
+;; FIXME: Use a `sketch-' prefix for all definitions.
+(defun sketch--svg-translate (dx dy)
   (interactive)
   (mapcar (lambda (node)
             (pcase (car node)
@@ -607,7 +616,7 @@ values"
     (transient-quit-one)
     (switch-to-buffer-other-window buffer)
     (erase-buffer)
-    (pp svg-sketch (current-buffer)))
+    (pp sketch (current-buffer)))
     (emacs-lisp-mode))

 (transient-define-suffix sketch-copy-definition ()
@@ -621,9 +630,10 @@ values"
   (interactive)
   (setq svg-sketch (read (buffer-string))))

+(defvar sketch-undo-redo nil)
+
 (transient-define-suffix sketch-undo ()
   (interactive)
-  (defvar sketch-undo-redo nil)
   (let ((sketch-reverse (nreverse svg-sketch)))
     (push (pop sketch-reverse) sketch-undo-redo)
     (setq svg-sketch (nreverse sketch-reverse)))
@@ -660,7 +670,7 @@ values"
                              ;;               (if sketch-include-end-marker
                              ;;                   "url(#arrow)"
                              ;;                 "none"))))
-    (apply 'svg-text svg-sketch text :x (car coords) :y (cdr coords) object-props))
+    (apply #'svg-text svg-sketch text :x (car coords) :y (cdr coords) object-props))
     (sketch-redraw))

 (transient-define-infix sketch-select-font ()
@@ -702,7 +712,7 @@ values"
     (setq svg-canvas (svg-create new-width new-height :stroke "gray"))
     (svg-marker svg-canvas "arrow" 8 8 "black" t)
     (svg-rectangle svg-canvas 0 0 new-width new-height :fill "white")
-    (setf (cddr svg-sketch) (svg-translate (car start-coords) (cdr start-coords)))
+    (setf (cddr svg-sketch) (sketch--svg-translate (car start-coords) (cdr start-coords)))
     (sketch-redraw)))

 (transient-define-suffix sketch-save ()
@@ -710,4 +720,4 @@ values"
   (image-save))

 (provide 'sketch-mode)
-;;; filename ends here
+;;; sketch-mode.el ends here
diff --git a/sketch-scratch.el b/sketch-scratch.el
index c9f60b21ad..6bf62552a7 100644
--- a/sketch-scratch.el
+++ b/sketch-scratch.el
@@ -1,3 +1,7 @@
+;;; sketch-scratch.el ---    -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2021  Free Software Foundation, Inc.
+
 ;; (setq svg-scratch (svg-create 100 100))
 ;; (svg-rectangle svg-scratch 25 25 50 50 :id "a")
 ;; (svg-line svg-scratch 25 25 75 75 :id "b" :stroke-color "black")
@@ -10,6 +14,7 @@
   (dolist (coord args node)
     (cl-decf (alist-get coord (cadr node)) amount)))

+;; FIXME: Use a `sketch-' prefix for all definitions.
 (defun svg-translate (dx dy)
   (interactive)
   (mapcar (lambda (node)