Thank you Ihor for the code review.
Now please help me find everything that can be improved in this patch.
> The patch looks good in general, but you need to add proper commit
> message. See https://orgmode.org/worg/
> <https://orgmode.org/worg/org-contribute.html#commit-messages>
> org-contribute.html#commit-messages
> <https://orgmode.org/worg/org-contribute.html#commit-messages>
Fixed.
Also, you need to add etc/ORG-NEWS entry about the new functionality and
> also modify the manual.
>
I added description to the manual and missing description of two
other commands: org-columns-move-left, org-columns-move-right that were
missing.
> Finally, I see no records about you copyright assignment status.
> Please take a look at
> https://orgmode.org/worg/org-contribute.html#copyright
>
Yes, it's my first time. I sent an email to ass...@gnu.org, yesterday.
Does this test have anything to do with the new feature?
>
Yes, you are right. This test has nothing in common with the feature. I
have removed it.
On Fri, Apr 7, 2023 at 1:01 PM Ihor Radchenko <yanta...@posteo.net> wrote:
> Sławomir Grochowski <slawomir.grochow...@gmail.com> writes:
>
> > Recently I often use 'column view' feature.
> > To my suprise in 'column view' user can't move rows up & down.
> > So I wrote a little code snippet to be able to do it, and I'm sharing it
> with you.
> >
> > Questions:
> > 1. Why user can't move rows up & down in 'column view'?
> > 2. Is this was intentional design decision?
>
> I do not see any particular reason.
> The current design dates back to 15 years ago - the initial commit in
> our current git repo.
>
> > I think 'column view' is missing one the core feel & functionality of
> org-mode - moving rows (headings) up & down.
> > In my experiance with 'column view' & tables I shuffle a lot of columns
> & rows order.
>
> Sounds reasonable.
>
> > From 1f0f2052b8dddf4982ab35267ed1564f2250784b Mon Sep 17 00:00:00 2001
> > From: Sławomir Grochowski <slawomir.grochow...@gmail.com>
> > Date: Mon, 3 Apr 2023 19:23:09 +0200
> > Subject: [PATCH] org-columns: add feat to move row up/down
>
> The patch looks good in general, but you need to add proper commit
> message. See https://orgmode.org/worg/org-contribute.html#commit-messages
>
> Also, you need to add etc/ORG-NEWS entry about the new functionality and
> also modify the manual.
>
> Finally, I see no records about you copyright assignment status.
> Please take a look at
> https://orgmode.org/worg/org-contribute.html#copyright
>
> > +(defun org-columns--move-row (&optional up)
> > + "Move table row. Calls `org-move-subtree-down' or
> `org-move-subtree-up'."
>
> *Move column view table row.
>
> We generally prefer single sentence as the first line of the docstring.
> Also, please describe UP argument in the docstring.
>
> > +;; Each column is an overlay on top of a character. So there has
> > +;; to be at least as many characters available on the line as
> > +;; columns to display.
> > +;; 'org-columns--display-here'
> > +(ert-deftest test-org-colview/bug-add-whitespace ()
> > + "Insert space characters if number of characters on the line
> > + is lower then number of columns."
> > + :expected-result :failed
>
> Does this test have anything to do with the new feature?
>
> > +(ert-deftest test-org-colview/columns-move-row-down ()
> > + "Test `org-columns-move-row-down' specifications."
> > + (should
> > + (equal "* H
> > +** B
> > +** A
> > +"
> > + (org-test-with-temp-text "* H
> > +** A
> > +** B
> > +"
> > + (let ((org-columns-default-format "%ITEM")) (org-columns)
> > + (next-line 1)
> > + (org-columns-move-row-down)
> > + (buffer-substring-no-properties (point-min)
> (point-max)))))))
>
> One special case we may want to consider is when columns are from
> different heading levels, like
>
> * H
> ** A
> *** A1
> ** B
>
> --
> Ihor Radchenko // yantar92,
> Org mode contributor,
> Learn more about Org mode at <https://orgmode.org/>.
> Support Org development at <https://liberapay.com/org-mode>,
> or support my work at <https://liberapay.com/yantar92>
>
From c5cc24607306399d1b1ca583e63a2fe7b71dbf89 Mon Sep 17 00:00:00 2001
From: Sławomir Grochowski <slawomir.grochow...@gmail.com>
Date: Mon, 3 Apr 2023 19:23:09 +0200
Subject: [PATCH] lisp/org-colview.el: add new commands to move column view
table row
* doc/org-manual.org (org-columns-move-row-up,
org-columns-move-row-down) and also (org-columns-move-left,
org-columns-move-right): Document two new and two old commands.
* etc/ORG-NEWS new commands to move rows up & down: Document the new feature.
* lisp/org-colview.el (org-columns--move-row, org-columns-move-row-up,
org-columns-move-row-down): New functions.
* testing/lisp/test-org-colview.el (test-org-colview/columns-move-row-down,
test-org-colview/columns-move-row-up,
test-org-colview/columns--move-row-stay-at-the-same-column,
test-org-colview/columns-move-row-down-with-subheading): New tests.
---
doc/org-manual.org | 24 ++++++++++++
etc/ORG-NEWS | 5 +++
lisp/org-colview.el | 24 ++++++++++++
testing/lisp/test-org-colview.el | 65 ++++++++++++++++++++++++++++++++
4 files changed, 118 insertions(+)
diff --git a/doc/org-manual.org b/doc/org-manual.org
index 50662669e..d5694df42 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -5761,6 +5761,30 @@ either for all clocks or just for today.
#+findex: org-columns-delete
Delete the current column.
+- {{{kbd(M-LEFT)}}} (~org-columns-move-left~) ::
+
+ #+kindex: M-LEFT
+ #+findex: org-columns-move-left
+ Move the current column left.
+
+- {{{kbd(M-RIGHT)}}} (~org-columns-move-right~) ::
+
+ #+kindex: M-RIGHT
+ #+findex: org-columns-move-right
+ Move the current column right.
+
+- {{{kbd(M-UP)}}} (~org-columns-move-row-up~) ::
+
+ #+kindex: M-UP
+ #+findex: org-columns-move-row-up
+ Move the current row up.
+
+- {{{kbd(M-DOWN)}}} (~org-columns-move-row-down~) ::
+
+ #+kindex: M-DOWN
+ #+findex: org-columns-move-row-down
+ Move the current row down.
+
*** Capturing column view
:PROPERTIES:
:DESCRIPTION: A dynamic block for column view.
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index ac233a986..438b3e7aa 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -362,6 +362,11 @@ After:
#+end_src
** New features
+*** Column view: new commands to move rows up & down
+You can move rows up & down in column view with
+~org-columns-move-row-up~ and ~org-columns-move-row-down~.
+Keybindings are the same as ~org-move-subtree-up~ and ~org-move-subtree-down~
+=M-<up>= and =M-<down>=.
*** Clock table can now produce quarterly reports
=:step= clock table parameter can now be set to =quarter=.
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 92a3b473d..1ce4d004b 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -209,6 +209,8 @@ See `org-columns-summary-types' for details.")
(org-defkey org-columns-map ">" #'org-columns-widen)
(org-defkey org-columns-map [(meta right)] #'org-columns-move-right)
(org-defkey org-columns-map [(meta left)] #'org-columns-move-left)
+(org-defkey org-columns-map [(meta down)] #'org-columns-move-row-down)
+(org-defkey org-columns-map [(meta up)] #'org-columns-move-row-up)
(org-defkey org-columns-map [(shift meta right)] #'org-columns-new)
(org-defkey org-columns-map [(shift meta left)] #'org-columns-delete)
(dotimes (i 10)
@@ -230,6 +232,8 @@ See `org-columns-summary-types' for details.")
"--"
["Move column right" org-columns-move-right t]
["Move column left" org-columns-move-left t]
+ ["Move row up" org-columns-move-row-up t]
+ ["Move row down" org-columns-move-row-down t]
["Add column" org-columns-new t]
["Delete column" org-columns-delete t]
"--"
@@ -1003,6 +1007,26 @@ details."
(org-columns-move-right)
(backward-char 1)))
+(defun org-columns--move-row (&optional up)
+ "Move the current table row down. With arg UP, move it up."
+ (let ((inhibit-read-only t)
+ (col (current-column)))
+ (if up (org-move-subtree-up)
+ (org-move-subtree-down))
+ (let ((org-columns-inhibit-recalculation t))
+ (org-columns-redo)
+ (move-to-column col))))
+
+(defun org-columns-move-row-down ()
+ "Move the current table row down."
+ (interactive)
+ (org-columns--move-row))
+
+(defun org-columns-move-row-up ()
+ "Move the current table row up."
+ (interactive)
+ (org-columns--move-row 'up))
+
(defun org-columns-store-format ()
"Store the text version of the current columns format.
The format is stored either in the COLUMNS property of the node
diff --git a/testing/lisp/test-org-colview.el b/testing/lisp/test-org-colview.el
index 9daec18e2..3985e8693 100644
--- a/testing/lisp/test-org-colview.el
+++ b/testing/lisp/test-org-colview.el
@@ -1010,6 +1010,71 @@
(list (get-char-property 1 'org-columns-value-modified)
(get-char-property 2 'org-columns-value-modified))))))
+(ert-deftest test-org-colview/columns-move-row-down ()
+ "Test `org-columns-move-row-down' specifications."
+ (should
+ (equal "* H
+** B
+** A
+"
+ (org-test-with-temp-text "* H
+** A
+** B
+"
+ (let ((org-columns-default-format "%ITEM")) (org-columns)
+ (next-line 1)
+ (org-columns-move-row-down)
+ (buffer-substring-no-properties (point-min) (point-max)))))))
+
+(ert-deftest test-org-colview/columns-move-row-up ()
+ "Test `org-columns-move-row-up' specifications."
+ (should
+ (equal "* H
+** B
+** A
+"
+ (org-test-with-temp-text "* H
+** A
+** B
+"
+ (let ((org-columns-default-format "%ITEM")) (org-columns)
+ (next-line 2)
+ (org-columns-move-row-up)
+ (buffer-substring-no-properties (point-min) (point-max)))))))
+
+(ert-deftest test-org-colview/columns--move-row-stay-at-the-same-column ()
+ "After function call 'org-columns--move-row' point should stay at the same column."
+ (should
+ (equal 35
+ (org-test-with-temp-text "* H
+** A
+** B
+"
+ (org-columns)
+ (next-line 1)
+ (forward-char 2)
+ (org-columns--move-row)
+ (current-column)))))
+
+(ert-deftest test-org-colview/columns-move-row-down-with-subheading ()
+ "Test `org-columns-move-row-up' specifications with subheading."
+ (should
+ (equal "* H
+** B
+** A
+*** A1
+"
+
+ (org-test-with-temp-text "* H
+** A
+*** A1
+** B
+"
+ (let ((org-columns-default-format "%ITEM")) (org-columns)
+ (next-line 1)
+ (org-columns-move-row-down)
+ (buffer-substring-no-properties (point-min) (point-max)))))))
+
(ert-deftest test-org-colview/columns-move-left ()
"Test `org-columns-move-left' specifications."
;; Error when trying to move the left-most column.
--
2.30.2
