[Patch] Draw and scribble notes in GNU Emacs

[Patch] Draw and scribble notes in GNU Emacs

[Anand Tamariya]

[-- Attachment #1.1: Type: text/plain, Size: 963 bytes --]

Here's a proposal for drawing and scribbling notes in GNU Emacs.
*Features:*

   - Draw simple shapes and text
   - Scribble freehand
   - Visual selection using mouse
   - Zoom
   - Undo/delete selection
   - Erase

*Code: https://gitlab.com/atamariya/emacs/-/blob/dev/lisp/svg.el
<https://gitlab.com/atamariya/emacs/-/blob/dev/lisp/svg.el>*
*Screenshots:*
https://lifeofpenguin.blogspot.com/2021/08/scribble-notes-in-gnu-emacs.html
*Video:* https://youtu.be/oj2OR5ytHx

I have few questions:
- Can this be included in Emacs core?
- I've copied two functions from Lars (
https://github.com/larsmagne/ewp/blob/master/ewp.el ). How should
attribution work in this case?

Additionally, following are mandatory patches for svg.el:
- polyline point uses CSV pairs separated by space.
- id should ideally be unique. This will allow changing element type when
different element with same id is added.
- If value is nil, don't include the attribute in the print output.

[-- Attachment #1.2: Type: text/html, Size: 1540 bytes --]

[-- Attachment #2: diff.txt --]
[-- Type: text/plain, Size: 1741 bytes --]

diff --git a/lisp/svg.el b/lisp/svg.el
index 7aadbc2359..45870e0bfe 100644
--- a/lisp/svg.el
+++ b/lisp/svg.el
@@ -154,9 +154,9 @@ svg-polyline
    (dom-node
     'polyline
     `((points . ,(mapconcat (lambda (pair)
-			      (format "%s %s" (car pair) (cdr pair)))
+			      (format "%s,%s" (car pair) (cdr pair)))
 			    points
-			    ", "))
+			    " "))
       ,@(svg--arguments svg args)))))
 
 (defun svg-polygon (svg points &rest args)
@@ -214,16 +214,17 @@ svg--encode-text
     (buffer-string)))
 
 (defun svg--append (svg node)
+  ;; id is expected to be unique.
   (let ((old (and (dom-attr node 'id)
 		  (dom-by-id svg
                              (concat "\\`" (regexp-quote (dom-attr node 'id))
                                      "\\'")))))
     (if old
-        ;; FIXME: This was (dom-set-attributes old (dom-attributes node))
-        ;; and got changed by commit f7ea7aa11f6211b5142bbcfc41c580d75485ca56
-        ;; without any explanation.
-	(setcdr (car old) (cdr node))
-      (dom-append-child svg node)))
+        ;; Remove old node. New node might be a different type.
+        (mapc (lambda (a)
+                  (dom-remove-node svg a))
+                old))
+    (dom-append-child svg node))
   (svg-possibly-update-image svg))
 
 (defun svg--image-data (image image-type datap)
@@ -311,7 +312,8 @@ svg-print
     (insert (format "<%s" (car dom)))
     (dolist (attr (nth 1 dom))
       ;; Ignore attributes that start with a colon.
-      (unless (= (aref (format "%s" (car attr)) 0) ?:)
+      (unless (or (= (aref (format "%s" (car attr)) 0) ?:)
+                  (null (cdr attr)))
         (insert (format " %s=\"%s\"" (car attr) (cdr attr)))))
     (insert ">")
     (dolist (elem (nthcdr 2 dom))

Re: [Patch] Draw and scribble notes in GNU Emacs

[Po Lu]

Anand Tamariya <atamariya@gmail.com> writes:

> Code: https://gitlab.com/atamariya/emacs/-/blob/dev/lisp/svg.el

Good work, but it appears to be comprised solely of modifications to
svg.el.  Is there any particular reason it cannot be separated from the
rest of svg.el, which is geared towards programmatic creation of images,
as opposed to providing a paint program?

  . ;; ewp functions copied from https://github.com/larsmagne/ewp/blob/master/ewp.el
  . (defun ewp-crop-image (&optional square)
  . (defun ewp-crop-image-update (area data size type)
  . (defun ewp-crop-image-1 (svg &optional square image-width image-height)
  . (defun ewp-find-corner (area pos corners)
  . (defun ewp-content-type (image)
  . (defun ewp--image-type ()

It would be good to prefix the names of these functions with something
other than "ewp", because they could conflict with the functions present
in ewp.

Further, I don't believe ewp has undergone copyright assignment to the
FSF.  If the code is to be included in Emacs, that will likely have to
be solved first.

Re: [Patch] Draw and scribble notes in GNU Emacs

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 09 Sep 2021 16:33:08 +0800
> 
>   . ;; ewp functions copied from https://github.com/larsmagne/ewp/blob/master/ewp.el
>   . (defun ewp-crop-image (&optional square)
>   . (defun ewp-crop-image-update (area data size type)
>   . (defun ewp-crop-image-1 (svg &optional square image-width image-height)
>   . (defun ewp-find-corner (area pos corners)
>   . (defun ewp-content-type (image)
>   . (defun ewp--image-type ()
> 
> It would be good to prefix the names of these functions with something
> other than "ewp", because they could conflict with the functions present
> in ewp.
> 
> Further, I don't believe ewp has undergone copyright assignment to the
> FSF.

If that is Lars, there should be no problem with his existing
assignment (assuming he agrees to include that code in Emacs).

Re: [Patch] Draw and scribble notes in GNU Emacs

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> If that is Lars, there should be no problem with his existing
> assignment (assuming he agrees to include that code in Emacs).

It does seem to be Lars, which is a nice thing.  I wonder what he thinks
about this?

Re: [Patch] Draw and scribble notes in GNU Emacs

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

>> If that is Lars, there should be no problem with his existing
>> assignment (assuming he agrees to include that code in Emacs).
>
> It does seem to be Lars, which is a nice thing.  I wonder what he thinks
> about this?

That I should push ewp to GNU ELPA.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [Patch] Draw and scribble notes in GNU Emacs

[dalanicolai]

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

Hey Anand,

this looks cool indeed! Have you seen my announcement of sketch-mode (
https://github.com/dalanicolai/sketch-mode) here?
I think you should check it out, it contains some nice ideas too.
Furthermore, I think your work could be well combined with my work into a
single package.
At first sight, it looks like we have been doing quite complementary
things. Would you be interested in joining efforts? I did not yet have a
closer look at your code b.t.w.,
just I thought it would be handy to notify you about sketch mode right away
(to prevent duplicate work, and to exchange ideas).

It looks like I am using a more updated version of svg.el b.t.w., maybe
you'd like to download and use the latest version of svg.el too.

Best regards,
Daniel



On Thu, 9 Sept 2021 at 16:51, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Po Lu <luangruo@yahoo.com> writes:
>
> >> If that is Lars, there should be no problem with his existing
> >> assignment (assuming he agrees to include that code in Emacs).
> >
> > It does seem to be Lars, which is a nice thing.  I wonder what he thinks
> > about this?
>
> That I should push ewp to GNU ELPA.  :-)
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>
>

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

Re: [Patch] Draw and scribble notes in GNU Emacs

[Anand Tamariya]

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

>
> this looks cool indeed! Have you seen my announcement of sketch-mode (
> https://github.com/dalanicolai/sketch-mode) here?
> I think you should check it out, it contains some nice ideas too.
> Furthermore, I think your work could be well combined with my work into a
> single package.
> At first sight, it looks like we have been doing quite complementary
> things. Would you be interested in joining efforts? I did not yet have a
> closer look at your code b.t.w.,
> just I thought it would be handy to notify you about sketch mode right
> away (to prevent duplicate work, and to exchange ideas).
>
I learnt about your package after I posted about mine on reddit. I agree we
have similar ideas. We only differ on the UI - you're using transient while
mine is geared towards taking quick notes. With my package, you continue to
use the keyboard. You switch mode only when you want to draw or you feel
it's easier to scribble. I believe both can co-exist.

It looks like I am using a more updated version of svg.el b.t.w., maybe
> you'd like to download and use the latest version of svg.el too.
>
Which version is this?

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

Re: [Patch] Draw and scribble notes in GNU Emacs

[Anand Tamariya]

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

On Thu, Sep 9, 2021 at 2:03 PM Po Lu <luangruo@yahoo.com> wrote:

> Anand Tamariya <atamariya@gmail.com> writes:
>
> > Code: https://gitlab.com/atamariya/emacs/-/blob/dev/lisp/svg.el
>
> Good work, but it appears to be comprised solely of modifications to
> svg.el.  Is there any particular reason it cannot be separated from the
> rest of svg.el, which is geared towards programmatic creation of images,
> as opposed to providing a paint program?
>
Ultimately, it will be a separate package. Right now it also includes some
critical patches to svg.el. Hence it's easier to distribute it this way for
early adopters.

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

Re: [Patch] Draw and scribble notes in GNU Emacs

[Lars Ingebrigtsen]

Anand Tamariya <atamariya@gmail.com> writes:

> Here's a proposal for drawing and scribbling notes in GNU Emacs.
> Features:
>
> * Draw simple shapes and text
> * Scribble freehand
> * Visual selection using mouse
> * Zoom
> * Undo/delete selection
> * Erase

Looks cool -- perhaps something for GNU ELPA?

> I have few questions:
> - Can this be included in Emacs core?
> - I've copied two functions from Lars
> (https://github.com/larsmagne/ewp/blob/master/ewp.el ). How should
> attribution work in this case?

I've been thinking that I should push ewp to GNU ELPA, but I've never
taken the time.  (It's a package for editing Wordpress blogs.)

>  (defun svg--append (svg node)
> +  ;; id is expected to be unique.
>    (let ((old (and (dom-attr node 'id)
>  		  (dom-by-id svg
>                               (concat "\\`" (regexp-quote (dom-attr node 'id))
>                                       "\\'")))))
>      (if old
> -        ;; FIXME: This was (dom-set-attributes old (dom-attributes node))
> -        ;; and got changed by commit f7ea7aa11f6211b5142bbcfc41c580d75485ca56
> -        ;; without any explanation.
> -	(setcdr (car old) (cdr node))
> -      (dom-append-child svg node)))
> +        ;; Remove old node. New node might be a different type.
> +        (mapc (lambda (a)
> +                  (dom-remove-node svg a))
> +                old))
> +    (dom-append-child svg node))

Hm...  I think this is basically correct, but it's a behaviour change --
previously svg--append wouldn't append if the ID already existed, but
would keep the save place in the structure, but with this change, it
really always appends.

So I don't think that's quite right -- the old behaviour was OK, I think
(but undocumented).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [Patch] Draw and scribble notes in GNU Emacs

[Anand Tamariya]

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

>
> >  (defun svg--append (svg node)
> > +  ;; id is expected to be unique.
> >    (let ((old (and (dom-attr node 'id)
> >                 (dom-by-id svg
> >                               (concat "\\`" (regexp-quote (dom-attr node
> 'id))
> >                                       "\\'")))))
> >      (if old
> > -        ;; FIXME: This was (dom-set-attributes old (dom-attributes
> node))
> > -        ;; and got changed by commit
> f7ea7aa11f6211b5142bbcfc41c580d75485ca56
> > -        ;; without any explanation.
> > -     (setcdr (car old) (cdr node))
> > -      (dom-append-child svg node)))
> > +        ;; Remove old node. New node might be a different type.
> > +        (mapc (lambda (a)
> > +                  (dom-remove-node svg a))
> > +                old))
> > +    (dom-append-child svg node))
>
> Hm...  I think this is basically correct, but it's a behaviour change --
> previously svg--append wouldn't append if the ID already existed, but
> would keep the save place in the structure, but with this change, it
> really always appends.
>
> So I don't think that's quite right -- the old behaviour was OK, I think
> (but undocumented).
>
Existing code doesn't ensure ids are unique. Furthermore, in case id is
duplicated, only the attributes on the first tag gets replaced. That's a
little unpredictable behaviour from an API perspective.

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

Re: [Patch] Draw and scribble notes in GNU Emacs

[Lars Ingebrigtsen]

Anand Tamariya <atamariya@gmail.com> writes:

> Existing code doesn't ensure ids are unique.

svg--append isn't the function to ensure this.  It's a very confusing
interface if a function like svg--append would make other nodes suddenly
disappear.

(And besides, `dom-remove-node' is very slow, so it shouldn't be used in
a function like this.)

> Furthermore, in case id is duplicated, only the attributes on the
> first tag gets replaced. That's a little unpredictable behaviour from
> an API perspective.

It's unpredictable, but the new version is still unpredictable -- but in
a different way, so I don't see a net positive here.  We're altering the
behaviour from one slightly odd interface to another slightly odd
interface.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [Patch] Draw and scribble notes in GNU Emacs

[Anand Tamariya]

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

> > Existing code doesn't ensure ids are unique.
>
> svg--append isn't the function to ensure this.

This is an internal function. So it's safe to change its name to something
which describes it's behaviour. However, this is the right function to
change as this is being called by all other svg node creation functions.

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

Re: [Patch] Draw and scribble notes in GNU Emacs

[Lars Ingebrigtsen]

Anand Tamariya <atamariya@gmail.com> writes:

>  > Existing code doesn't ensure ids are unique.
>
>  svg--append isn't the function to ensure this. 
>
> This is an internal function. So it's safe to change its name to something
> which describes it's behaviour. However, this is the right function to change
> as this is being called by all other svg node creation functions.

The name isn't the point -- you're changing the way all these functions
work in an incompatible way.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no