emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
@ 2023-04-06 10:11 Sławomir Grochowski
  2023-04-07 11:04 ` Ihor Radchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Sławomir Grochowski @ 2023-04-06 10:11 UTC (permalink / raw)
  To: emacs-orgmode

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

Dear All,

This is my first proposal & code submit. 
I will be very gratefull for you comments & feedback what can be done better.  

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

In my point of view 'core fuctionalities of org-mode' are: 
1. moving 'things' headings, rows, columns
   1. horizontally (left-right)
   2. vertically (up-down)
2. cyclic visibility

Let's compare this functionality in 3 areas:  

|                   | heading | table | column view |
|-------------------+---------+-------+-------------|
| moving left-right | [X]     | [X]   | [X]         |
| moving up-down    | [X]     | [X]   | [ ]         |
| cyclic visibility | [X]     | NA    | [X]         |

Cyclic visibility does not apply to 'table' because it operate on simple data, which is not structured, nested.

'Column view' transform 'headings' into rows, and their properties into columns, creating a table.

'Column view' is a combination of 'table' & 'heading':
'Heading' gives a 'table' properties for columns and cyclic visibilty feature, because it provide nested structure.
'Table' gives a 'heading' a 'column view' on heading properties.

'Column view' is similar to 'table'. They operate on same actions & keybindings.

|                             | column view      | table            |
|-----------------------------+------------------+------------------|
| move column left-right      | [X]              | [X]              |
| move column left-right keys | meta left-right  | meta left-right  |
| move row up-down            | [ ]              | [X]              |
| move row up-down keys       |                  | meta up-down     |
| add column                  | [X]              | [X]              |
| add column keys             | shift-meta right | shift-meta right |
| remove column               | [X]              | [X]              |
| remove column keys          | shift-meta left  | shift-meta left  |

I thnik it would be beneficial to have consistance interface in 'column view' and 'table'.

Based on that, I propose to add functionality to 'move row up/down' in 'column view' and set keybinding the same as in table above -> meta up/down.
Patch is in the attachment.

What others think about it? 

Regards,
Sławomir Grochowski

[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4416 bytes --]

From 1f0f2052b8dddf4982ab35267ed1564f2250784b Mon Sep 17 00:00:00 2001
From: Sławomir Grochowski <slawomir.grochowski@gmail.com>
Date: Mon, 3 Apr 2023 19:23:09 +0200
Subject: [PATCH] org-columns: add feat to move row up/down

---
 lisp/org-colview.el              | 22 +++++++++++
 testing/lisp/test-org-colview.el | 66 ++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 92a3b473d..0971d5eef 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)
@@ -1003,6 +1005,26 @@ details."
     (org-columns-move-right)
     (backward-char 1)))
 
+(defun org-columns--move-row (&optional up)
+    "Move table row. Calls `org-move-subtree-down' or `org-move-subtree-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 table row (subtree) down."
+  (interactive)
+  (org-columns--move-row))
+
+(defun org-columns-move-row-up ()
+  "Move table row (subtree) 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..fd02f054c 100644
--- a/testing/lisp/test-org-colview.el
+++ b/testing/lisp/test-org-colview.el
@@ -1010,6 +1010,72 @@
 	    (list (get-char-property 1 'org-columns-value-modified)
 		  (get-char-property 2 'org-columns-value-modified))))))
 
+;; 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
+  (should
+   (equal "* H
+** A
+** B
+"
+          (org-test-with-temp-text "* H
+** A
+** B
+"
+            (org-columns)
+            (buffer-substring-no-properties (point-min) (point-max))))))
+
+(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 ()
+  "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-down)
+          (current-column)))))
+
 (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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-04-06 10:11 [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down Sławomir Grochowski
@ 2023-04-07 11:04 ` Ihor Radchenko
  2023-04-08 21:48   ` Sławomir Grochowski
  0 siblings, 1 reply; 9+ messages in thread
From: Ihor Radchenko @ 2023-04-07 11:04 UTC (permalink / raw)
  To: Sławomir Grochowski; +Cc: emacs-orgmode

Sławomir Grochowski <slawomir.grochowski@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.grochowski@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>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-04-07 11:04 ` Ihor Radchenko
@ 2023-04-08 21:48   ` Sławomir Grochowski
  2023-04-09  9:16     ` Ihor Radchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Sławomir Grochowski @ 2023-04-08 21:48 UTC (permalink / raw)
  To: emacs-orgmode


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

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 assign@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 <yantar92@posteo.net> wrote:

> Sławomir Grochowski <slawomir.grochowski@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.grochowski@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>
>

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

[-- Attachment #2: 0001-lisp-org-colview.el-add-new-commands-to-move-column-.patch --]
[-- Type: text/x-patch, Size: 6795 bytes --]

From c5cc24607306399d1b1ca583e63a2fe7b71dbf89 Mon Sep 17 00:00:00 2001
From: Sławomir Grochowski <slawomir.grochowski@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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-04-08 21:48   ` Sławomir Grochowski
@ 2023-04-09  9:16     ` Ihor Radchenko
  2023-04-11  8:49       ` Sławomir Grochowski
  0 siblings, 1 reply; 9+ messages in thread
From: Ihor Radchenko @ 2023-04-09  9:16 UTC (permalink / raw)
  To: Sławomir Grochowski; +Cc: emacs-orgmode

Sławomir Grochowski <slawomir.grochowski@gmail.com> writes:

> Thank you Ihor for the code review.
> Now please help me find everything that can be improved in this patch.

I just tried to use the patch using the following test case:

1. Create a new Org file containing

* A 
* B 
* C 
** C1
* D 

2. C-u C-c C-x C-c
3. M-<
4. M-<down> M-<up> M-<down> M-<up> M-<down> M-<up> M-<down> M-<up> C-c C-c
5. Observe text disappearing and columns being unhighlighted. At the
   end, the buffer is left with

* B

>> 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 assign@gnu.org, yesterday.

Note that they should reply within 5 working days.

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-04-09  9:16     ` Ihor Radchenko
@ 2023-04-11  8:49       ` Sławomir Grochowski
  2023-04-12  9:09         ` Ihor Radchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Sławomir Grochowski @ 2023-04-11  8:49 UTC (permalink / raw)
  To: emacs-orgmode

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

Thank you Ihor for finding such an important bug.

I've tried to pinpoint it.
I think it happens only when: first heading (row) is the first line of the
buffer.
To reproduce it:
* a
* b
* c

C-u C-c C-x C-c => runs `org-columns' with global argument.
Using a keyboard. Press M-<down>, M-<down>, M-<up>.
Heading 'a' will disappear.

I've tried to create a test based on that behaviour, but can't reproduce
this bug in test env.
Also I can't reproduce this when I'm invoking the commands directly from
M-x.
As I understand it something with overlay. Which try to jump before the
first line of the buffer.
And that makes some kind of overflow?
I will try to simplify the example and create a simple overlay without
org-columns.
If anyone has some ideas how to debug it I will be grateful for the hints.


On Sun, Apr 9, 2023 at 11:13 AM Ihor Radchenko <yantar92@posteo.net> wrote:

> Sławomir Grochowski <slawomir.grochowski@gmail.com> writes:
>
> > Thank you Ihor for the code review.
> > Now please help me find everything that can be improved in this patch.
>
> I just tried to use the patch using the following test case:
>
> 1. Create a new Org file containing
>
> * A
> * B
> * C
> ** C1
> * D
>
> 2. C-u C-c C-x C-c
> 3. M-<
> 4. M-<down> M-<up> M-<down> M-<up> M-<down> M-<up> M-<down> M-<up> C-c C-c
> 5. Observe text disappearing and columns being unhighlighted. At the
>    end, the buffer is left with
>
> * B
>
> >> 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 assign@gnu.org, yesterday.
>
> Note that they should reply within 5 working days.
>
> --
> 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>
>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-04-11  8:49       ` Sławomir Grochowski
@ 2023-04-12  9:09         ` Ihor Radchenko
  2023-08-20 10:43           ` Ihor Radchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Ihor Radchenko @ 2023-04-12  9:09 UTC (permalink / raw)
  To: Sławomir Grochowski; +Cc: emacs-orgmode

Sławomir Grochowski <slawomir.grochowski@gmail.com> writes:

> As I understand it something with overlay. Which try to jump before the
> first line of the buffer.

Most likely.

> And that makes some kind of overflow?
> I will try to simplify the example and create a simple overlay without
> org-columns.
> If anyone has some ideas how to debug it I will be grateful for the hints.

I suspect that it is something with overlay boundaries. AFAIR, some org
subtree motion functions use `insert-before-markers', which may be
tricky when we insert right at the beginning/end of an overlay.

I suggest using M-x debug-on-entry and stepping through the code.
You may also add a random colour to colview overlays to track how they
behave visually. You can also examine `org-columns-overlays' variable
that holds all the active colview overlays.

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-04-12  9:09         ` Ihor Radchenko
@ 2023-08-20 10:43           ` Ihor Radchenko
  2023-08-20 17:44             ` Sławomir Grochowski
  0 siblings, 1 reply; 9+ messages in thread
From: Ihor Radchenko @ 2023-08-20 10:43 UTC (permalink / raw)
  To: Sławomir Grochowski; +Cc: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:

> I suspect that it is something with overlay boundaries. AFAIR, some org
> subtree motion functions use `insert-before-markers', which may be
> tricky when we insert right at the beginning/end of an overlay.

It was totally unrelated bug on Org side.
Applied, onto main, with minor amendments to function docstring and
changelog format.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=650e42996
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e08d87831

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-08-20 10:43           ` Ihor Radchenko
@ 2023-08-20 17:44             ` Sławomir Grochowski
  2023-08-20 19:39               ` Ihor Radchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Sławomir Grochowski @ 2023-08-20 17:44 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

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

I thought it would get lost in the many thousands of other TODOs you have
on your plate.
Thank you.

But to my another surprise you have fixed the bug that was killing me
everyday 'org-insert-heading: Fix when folded text is kept right at the new
heading'.
Thank you very much.

Regards,
Sławomir Grochowski


On Sun, Aug 20, 2023 at 12:43 PM Ihor Radchenko <yantar92@posteo.net> wrote:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
> > I suspect that it is something with overlay boundaries. AFAIR, some org
> > subtree motion functions use `insert-before-markers', which may be
> > tricky when we insert right at the beginning/end of an overlay.
>
> It was totally unrelated bug on Org side.
> Applied, onto main, with minor amendments to function docstring and
> changelog format.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=650e42996
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e08d87831
>
> --
> 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>
>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
  2023-08-20 17:44             ` Sławomir Grochowski
@ 2023-08-20 19:39               ` Ihor Radchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Ihor Radchenko @ 2023-08-20 19:39 UTC (permalink / raw)
  To: Sławomir Grochowski; +Cc: emacs-orgmode

Sławomir Grochowski <slawomir.grochowski@gmail.com> writes:

> I thought it would get lost in the many thousands of other TODOs you have
> on your plate.
> Thank you.

Merging contributed patches is high priority. I either have them
scheduled in agenda or they are the next thing to do upon clearing the
scheduled tasks.

The last thing I want is being a sole committer to Org.

> But to my another surprise you have fixed the bug that was killing me
> everyday 'org-insert-heading: Fix when folded text is kept right at the new
> heading'.
> Thank you very much.

A reproducer was provided :) So I was able to debug what was going on.

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-08-20 19:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-06 10:11 [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down Sławomir Grochowski
2023-04-07 11:04 ` Ihor Radchenko
2023-04-08 21:48   ` Sławomir Grochowski
2023-04-09  9:16     ` Ihor Radchenko
2023-04-11  8:49       ` Sławomir Grochowski
2023-04-12  9:09         ` Ihor Radchenko
2023-08-20 10:43           ` Ihor Radchenko
2023-08-20 17:44             ` Sławomir Grochowski
2023-08-20 19:39               ` Ihor Radchenko

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).