proposal: edit-rectangle

proposal: edit-rectangle

[John Wiegley]

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

This is functionality I'd like to add to rect.el. It allows you to specify a
rectangular region, and then edit just the contents of that rectangle in a
separate buffer. When finished (C-c C-c), it replaces the old rectangle using
insert-rectangle with the new contents.

I'd appreciate comments on usability, fitness, etc.

--8<---------------cut here---------------start------------->8---
(defvar edit-rectangle-origin)
(defvar edit-rectangle-saved-window-config)

(defun edit-rectangle (&optional start end)
  (interactive "r")
  (let ((strs (delete-extract-rectangle start end))
        (mode major-mode)
        (here (copy-marker (min (mark) (point)) t))
        (config (current-window-configuration)))
    (with-current-buffer (generate-new-buffer "*Rectangle*")
      (funcall mode)
      (set (make-local-variable 'edit-rectangle-origin) here)
      (set (make-local-variable 'edit-rectangle-saved-window-config) config)
      (local-set-key (kbd "C-c C-c") #'restore-rectangle)
      (mapc #'(lambda (x) (insert x ?\n)) strs)
      (goto-char (point-min))
      (pop-to-buffer (current-buffer)))))

(defun restore-rectangle ()
  (interactive)
  (let ((content (split-string (buffer-string) "\n"))
        (origin edit-rectangle-origin)
        (config edit-rectangle-saved-window-config))
    (with-current-buffer (marker-buffer origin)
      (goto-char origin)
      (insert-rectangle content))
    (kill-buffer (current-buffer))
    (set-window-configuration config)))
--8<---------------cut here---------------end--------------->8---

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

Re: proposal: edit-rectangle

[Kaushal Modi]

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

Thanks John,

I tried out that snippet. It works great, and as advertised. I think will
be a useful function.

Here are few notes I have on this:
- The functions need docstrings
- It will also be useful to document them in the manual ( (emacs)
Rectangles )
- I first wondered why you named it edit-rectangle instead of
rectangle-edit. But then I realized that all rectangle commands are named
that way: kill-rectangle, clear-rectangle, etc.. So that's why?

On Thu, Oct 6, 2016 at 1:08 PM John Wiegley <jwiegley@gmail.com> wrote:

> This is functionality I'd like to add to rect.el. It allows you to specify
> a
> rectangular region, and then edit just the contents of that rectangle in a
> separate buffer. When finished (C-c C-c), it replaces the old rectangle
> using
> insert-rectangle with the new contents.
>
> I'd appreciate comments on usability, fitness, etc.
>
> --

Kaushal Modi

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

Re: proposal: edit-rectangle

[John Wiegley]

>>>>> Kaushal Modi <kaushal.modi@gmail.com> writes:

> - The functions need docstrings
> - It will also be useful to document them in the manual ( (emacs)
> Rectangles )

Ah, of course. :)

> - I first wondered why you named it edit-rectangle instead of
> rectangle-edit. But then I realized that all rectangle commands are named
> that way: kill-rectangle, clear-rectangle, etc.. So that's why?

Yes, for people who use this function via M-x, I wanted it to fit with their
intuition based on how the other rectangle editing functions are named.

Personally I'd prefer the uniformity of rectangle-*, but I don't think this
function should be an outlier only for that reason.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: proposal: edit-rectangle

[Thierry Volpiatto]

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

John Wiegley <jwiegley@gmail.com> writes:

> This is functionality I'd like to add to rect.el. It allows you to specify a
> rectangular region, and then edit just the contents of that rectangle in a
> separate buffer. When finished (C-c C-c), it replaces the old rectangle using
> insert-rectangle with the new contents.
>
> I'd appreciate comments on usability, fitness, etc.

Looks cool, what about a more lexical variant (not tested) ?

--8<---------------cut here---------------start------------->8---
(defun edit-rectangle (&optional start end)
  (interactive "r")
  (let ((strs   (delete-extract-rectangle start end))
        (mode   major-mode)
        (here   (copy-marker (min (mark) (point)) t))
        (config (current-window-configuration)))
    (with-current-buffer (generate-new-buffer "*Rectangle*")
      (funcall mode)
      (local-set-key (kbd "C-c C-c") (lambda ()
                                       (interactive)
                                       (restore-rectangle-1 here config)))
      (mapc (lambda (x) (insert x ?\n)) strs)
      (goto-char (point-min))
      (pop-to-buffer (current-buffer)))))

(defun restore-rectangle-1 (origin config)
  (let ((content (split-string (buffer-string) "\n")))
    (with-current-buffer (marker-buffer origin)
      (goto-char origin)
      (insert-rectangle content))
    (kill-buffer (current-buffer))
    (set-window-configuration config)))
--8<---------------cut here---------------end--------------->8---

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 180 bytes --]

Re: proposal: edit-rectangle

[Thierry Volpiatto]

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

John Wiegley <jwiegley@gmail.com> writes:

> This is functionality I'd like to add to rect.el. It allows you to specify a
> rectangular region, and then edit just the contents of that rectangle in a
> separate buffer. When finished (C-c C-c), it replaces the old rectangle using
> insert-rectangle with the new contents.
>
> I'd appreciate comments on usability, fitness, etc.

Also, after inserting the new rectangle, undo should restore buffer as
before, just need for this to add `buffer-disable-undo` in first fn and
`buffer-enable-undo` in second fn.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 180 bytes --]

Re: proposal: edit-rectangle

[Dmitri Paduchikh]

John Wiegley <jwiegley@gmail.com> wrote:

JW> This is functionality I'd like to add to rect.el. It allows you to specify a
JW> rectangular region, and then edit just the contents of that rectangle in a
JW> separate buffer. When finished (C-c C-c), it replaces the old rectangle using
JW> insert-rectangle with the new contents.

JW> I'd appreciate comments on usability, fitness, etc.

There is one inconvenience: user is responsible for keeping all the
lines of rectangle of the equal length, otherwise the rectangular
structure at the destination breaks.

Regards,
Dmitri Paduchikh

Re: proposal: edit-rectangle

[John Mastro]

John Wiegley <jwiegley@gmail.com> wrote:
> This is functionality I'd like to add to rect.el. It allows you to specify a
> rectangular region, and then edit just the contents of that rectangle in a
> separate buffer. When finished (C-c C-c), it replaces the old rectangle using
> insert-rectangle with the new contents.
>
> I'd appreciate comments on usability, fitness, etc.

Neat idea, I like it.

> --8<---------------cut here---------------start------------->8---
> (defvar edit-rectangle-origin)
> (defvar edit-rectangle-saved-window-config)
>
> (defun edit-rectangle (&optional start end)
>   (interactive "r")
>   (let ((strs (delete-extract-rectangle start end))
>         (mode major-mode)
>         (here (copy-marker (min (mark) (point)) t))

Instead of mark and point, should this use the start and end arguments?

Also, since the number of live markers has performance implications, I
think you should (set-marker <marker> nil) once you're done with it in
`restore-rectangle'.

>         (config (current-window-configuration)))
>     (with-current-buffer (generate-new-buffer "*Rectangle*")
>       (funcall mode)
>       (set (make-local-variable 'edit-rectangle-origin) here)
>       (set (make-local-variable 'edit-rectangle-saved-window-config) config)

You could use `setq-local' here.

>       (local-set-key (kbd "C-c C-c") #'restore-rectangle)
>       (mapc #'(lambda (x) (insert x ?\n)) strs)
>       (goto-char (point-min))
>       (pop-to-buffer (current-buffer)))))
>
> (defun restore-rectangle ()
>   (interactive)
>   (let ((content (split-string (buffer-string) "\n"))
>         (origin edit-rectangle-origin)
>         (config edit-rectangle-saved-window-config))
>     (with-current-buffer (marker-buffer origin)
>       (goto-char origin)
>       (insert-rectangle content))
>     (kill-buffer (current-buffer))
>     (set-window-configuration config)))
> --8<---------------cut here---------------end--------------->8---

The results are a bit awkward if you `restore-rectangle' with more lines
than you started with, but I don't have any ideas offhand for
obviously better behavior.

        John

Re: proposal: edit-rectangle

[Alain Schneble]

John Wiegley <jwiegley@gmail.com> writes:

> This is functionality I'd like to add to rect.el. It allows you to specify a
> rectangular region, and then edit just the contents of that rectangle in a
> separate buffer. When finished (C-c C-c), it replaces the old rectangle using
> insert-rectangle with the new contents.

Thanks, that's a useful addition.  I usually like to be able to cancel
editing with C-c C-k in scenarios like this.  Would you find it useful
too?

Alain

Re: proposal: edit-rectangle

[Kaushal Modi]

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

On Thu, Oct 6, 2016 at 4:25 PM Alain Schneble <a.s@realize.ch> wrote:

> Thanks, that's a useful addition.  I usually like to be able to cancel
> editing with C-c C-k in scenarios like this.  Would you find it useful
> too?
>
>
+1
-- 

Kaushal Modi

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

Re: proposal: edit-rectangle

[John Wiegley]

>>>>> "AS" == Alain Schneble <a.s@realize.ch> writes:

AS> Thanks, that's a useful addition. I usually like to be able to cancel
AS> editing with C-c C-k in scenarios like this. Would you find it useful too?

You're right, I use that often as well. I think everyone's suggestions have
been good, and I will apply them. It remains an open question what should be
done for mismatching rectangle sizes, but I may have to leave that up to the
simplest interpretation (i.e., just stick with what kill/yank rectangle would
have done given the same scenario).

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: proposal: edit-rectangle

[Stefan Monnier]

> Personally I'd prefer the uniformity of rectangle-*, but I don't think this
> function should be an outlier only for that reason.

FWIW, Emacs has been slowly moving towards using the <prefix>-<command>
even for its "core" operations, and I'd be in favor of renaming all the
rectangle commands to rectangle-* (keeping obsolete aliases, obviously).

Part of the motivation for me is that we otherwise look like hypocrites
when we ask package authors to stick to a <prefix>-* naming convention.


        Stefan "same for abbrev.el"

Re: proposal: edit-rectangle

[Kaushal Modi]

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

On Thu, Oct 6, 2016 at 4:51 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > Personally I'd prefer the uniformity of rectangle-*, but I don't think
> this
> > function should be an outlier only for that reason.
>
> FWIW, Emacs has been slowly moving towards using the <prefix>-<command>
> even for its "core" operations, and I'd be in favor of renaming all the
> rectangle commands to rectangle-* (keeping obsolete aliases, obviously).
>
> Part of the motivation for me is that we otherwise look like hypocrites
> when we ask package authors to stick to a <prefix>-* naming convention.
>
>
>         Stefan "same for abbrev.el"
>

 +1 That's what I meant :)
-- 

Kaushal Modi

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

Re: proposal: edit-rectangle

[Stefan Monnier]

> (defvar edit-rectangle-origin)
> (defvar edit-rectangle-saved-window-config)

Since they're internal, I'd name them with a "rectangle--" prefix.
I'd also give them a docstring.

> (defun edit-rectangle (&optional start end)
>   (interactive "r")
>   (let ((strs (delete-extract-rectangle start end))

I would leave the original rectangle untouched until the user hits C-c C-c.

>         (mode major-mode)
>         (here (copy-marker (min (mark) (point)) t))
>         (config (current-window-configuration)))
>     (with-current-buffer (generate-new-buffer "*Rectangle*")
>       (funcall mode)
>       (set (make-local-variable 'edit-rectangle-origin) here)
>       (set (make-local-variable 'edit-rectangle-saved-window-config) config)

You can use setq-local here.

>       (local-set-key (kbd "C-c C-c") #'restore-rectangle)

This modifies the local keymap, i.e. the major mode's keymap (shared
by all other buffers using this major mode).

You could use (use-local-map `(keymap . ,(current-local-map))), but
I think I'd vote for a minor-mode map and actually define a matching
minor mode.  And make it (along with the above buffer-local vars)
permanent-local, so the user can switch major-mode.

>       (mapc #'(lambda (x) (insert x ?\n)) strs)

    (dolist (x strs) (insert x ?\n))

Indents better, runs faster, and might even generate more compact code
(haven't bothered to check, tho).  It only works if `strs` is a list
(whereas the `mapc` version also works for arrays).

> (defun restore-rectangle ()

"restore" makes it sound like you're reverting to an older state.
How 'bout `rectangle-edit-done` ?

>   (interactive)
>   (let ((content (split-string (buffer-string) "\n"))
>         (origin edit-rectangle-origin)
>         (config edit-rectangle-saved-window-config))
>     (with-current-buffer (marker-buffer origin)
>       (goto-char origin)
>       (insert-rectangle content))
>     (kill-buffer (current-buffer))
>     (set-window-configuration config)))

I don't like window-configurations, so I'd rather just use something
like bury-buffer.  One of the reasons is that in my kind of setup the
above code will fail: edit-rectangle's pop-to-buffer will have a created
a new frame, so calling set-window-configuration in that new frame at
best makes little sense, and in practice signals an error because
window-configs can only be applied to their original frame.


        Stefan

Re: proposal: edit-rectangle

[Alain Schneble]

John Wiegley <jwiegley@gmail.com> writes:

> This is functionality I'd like to add to rect.el. It allows you to specify a
> rectangular region, and then edit just the contents of that rectangle in a
> separate buffer. When finished (C-c C-c), it replaces the old rectangle using
> insert-rectangle with the new contents.
>
> I'd appreciate comments on usability, fitness, etc.

There's an edge-case when editing rectangular areas spawning beyond end
of lines.  In this situation 'edit-rectangle' followed by
'restore-rectangle' changes the selected area even though no explicit
changes have been committed to that area.  There are phantom whitespace
characters added to some lines.

This is a simple recipe:

(insert
 (with-temp-buffer
   (insert (mapconcat 'identity '("john" "improved" "rectangle" "editing" "capabilities" "of" "emacs") "\n"))
   (goto-char 4) ; joh_n
   (rectangle-mark-mode)
   (end-of-line 5) ; capabilities_
   (with-current-buffer (edit-rectangle (mark) (point))
     (restore-rectangle))
   (buffer-string)))

I know it's kind of inherent to fill lines with spaces where needed to
get a rectangular area.  But in this round-trip scenario, it feels not
natural to me.  Not sure if this can be considered a bug though.

(Strangely, in my Emacs it fills some lines with tabs AND spaces.  Not
sure where the tabs come from...)

Alain

Re: proposal: edit-rectangle

[Alain Schneble]

Alain Schneble <a.s@realize.ch> writes:

> There's an edge-case when editing rectangular areas spawning beyond end
                                                      ^^^^^^^^
Sorry, I meant areas that *span* beyond end of lines...

> of lines.  In this situation 'edit-rectangle' followed by
> 'restore-rectangle' changes the selected area even though no explicit
> changes have been committed to that area.  There are phantom whitespace
> characters added to some lines.

Alain

Re: proposal: edit-rectangle

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Also, after inserting the new rectangle, undo should restore buffer as
  > before, just need for this to add `buffer-disable-undo` in first fn and
  > `buffer-enable-undo` in second fn.

A smarter way to handle it is to scan the undo list at the end of
edit-rectangle, and convert those specific undo entries so that they
describe the same changes as having been made directly in the buffer.
That way, you could undo those individual changes one by one.

  > The results are a bit awkward if you `restore-rectangle' with more lines
  > than you started with, but I don't have any ideas offhand for
  > obviously better behavior.

Perhaps duplicate the last line of the surrounding text so that you have
enough lines to put the rectangle into.



-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.