emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/d


From: Sławomir Grochowski
Subject: Re: [RFC] [feat] org-colview/org-columns: 'column view' moving rows up/down
Date: Sat, 8 Apr 2023 23:48:19 +0200

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/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: 0001-lisp-org-colview.el-add-new-commands-to-move-column-.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]