Re: new package: sketch-mode

[dalanicolai <dalanicolai@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 9614 bytes --]

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)
>
>

[-- Attachment #2: Type: text/html, Size: 11964 bytes --]