From: Kaushal Modi <kaushal.modi@gmail.com>
To: emacs-org list <emacs-orgmode@gnu.org>
Subject: [PATCH] Refactor org-set-tags arguments for clarity
Date: Wed, 12 Jul 2017 02:46:05 +0000 [thread overview]
Message-ID: <CAFyQvY1aDNQgnwwtWaC00Z__okrDT_Y0RLbPko-_=tNc+pNZjA@mail.gmail.com> (raw)
[-- Attachment #1.1: Type: text/plain, Size: 1085 bytes --]
Hello,
I recently started looking at the org-set-tags function in org.el, but then
quickly got confused with the doc-string.
"Set the tags for the current headline.
With prefix ARG, realign all tags in headings in the current buffer.
When JUST-ALIGN is non-nil, only align tags."
The purpose of ARG and JUST-ALIGN seems to be the exact same from the
doc-string. On reading the code, I realized that actually ARG should have
been called JUST-ALIGN and the JUST-ALIGN should have been called
ALIGN-ONLY-CURRENT.
The attached patch contains the updated doc-string, refactoring of the
argument names, and renaming of the argument symbol to :align-only-current
from 'align and 'ignore-column in org-set-tag calls. I have left most of
the org-set-tags calls untouched where the argument values are simply t
instead of descriptive 'align or 'ignore-column.
As the patch introduces no functional changes, I have based it off maint.
"make test" is still passing with these changes.
Can you please review the patch and let me know if it's good for committing?
Thanks.
--
Kaushal Modi
[-- Attachment #1.2: Type: text/html, Size: 1504 bytes --]
[-- Attachment #2: 0001-Clarify-the-purpose-of-org-set-tags-arguments.patch --]
[-- Type: application/octet-stream, Size: 17279 bytes --]
From f4837c9231e36556a030b4e02dd92a8ac3522029 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Tue, 11 Jul 2017 22:40:38 -0400
Subject: [PATCH] Clarify the purpose of org-set-tags arguments
* lisp/org.el (org-set-tags-command, org-set-tags): Improve argument
names and doc-string. Auto-indent the functions.
* lisp/org-mouse.el (org-mouse-tag-menu): Remove redundant argument in
`org-set-tags' call.
* lisp/org.el (org-promote, org-demote, org-priority)
(org-set-tags-to, org-entry-put):
* lisp/org-mobile.el (org-mobile-edit):
* lisp/org-capture.el (org-capture-fill-template): Use a more accurate
argument symbol to match the above `org-set-tags' refactoring.
* lisp/org.el (org-fix-tags-on-the-fly): Typo fix in docstring.
---
lisp/org-capture.el | 2 +-
lisp/org-mobile.el | 2 +-
lisp/org-mouse.el | 2 +-
lisp/org.el | 258 ++++++++++++++++++++++++++++------------------------
4 files changed, 140 insertions(+), 124 deletions(-)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 1175cb4084..41d9e40d6d 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1728,7 +1728,7 @@ The template may still contain \"%?\" for cursor positioning."
(unless (eq (char-after) ?:) (insert ":"))
(and (org-at-heading-p)
(let ((org-ignore-region t))
- (org-set-tags nil 'align))))))
+ (org-set-tags nil :align-only-current))))))
((or "C" "L")
(let ((insert-fun (if (equal key "C") #'insert
(lambda (s) (org-insert-link 0 s)))))
diff --git a/lisp/org-mobile.el b/lisp/org-mobile.el
index 12e6c84b3c..065137c53d 100644
--- a/lisp/org-mobile.el
+++ b/lisp/org-mobile.el
@@ -1031,7 +1031,7 @@ be returned that indicates what went wrong."
(goto-char (match-beginning 4))
(insert new)
(delete-region (point) (+ (point) (length current)))
- (org-set-tags nil 'align))
+ (org-set-tags nil :align-only-current))
(t (error "Heading changed in MobileOrg and on the computer")))))))
((eq what 'addheading)
diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
index d6a472787e..d662f87673 100644
--- a/lisp/org-mouse.el
+++ b/lisp/org-mouse.el
@@ -435,7 +435,7 @@ SCHEDULED: or DEADLINE: or ANYTHINGLIKETHIS:"
))
'("--"
["Align Tags Here" (org-set-tags nil t) t]
- ["Align Tags in Buffer" (org-set-tags t t) t]
+ ["Align Tags in Buffer" (org-set-tags t) t]
["Set Tags ..." (org-set-tags) t])))
(defun org-mouse-set-tags (tags)
diff --git a/lisp/org.el b/lisp/org.el
index eaa85dd853..b2abccf0b6 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -8291,7 +8291,7 @@ even level numbers will become the next higher odd number."
(user-error "Cannot promote to level 0. UNDO to recover if necessary"))
(t (replace-match up-head nil t)))
(unless (= level 1)
- (when org-auto-align-tags (org-set-tags nil 'ignore-column))
+ (when org-auto-align-tags (org-set-tags nil :align-only-current))
(when org-adapt-indentation (org-fixup-indentation (- diff))))
(run-hooks 'org-after-promote-entry-hook))))
@@ -8305,7 +8305,7 @@ even level numbers will become the next higher odd number."
(down-head (concat (make-string (org-get-valid-level level 1) ?*) " "))
(diff (abs (- level (length down-head) -1))))
(replace-match down-head nil t)
- (when org-auto-align-tags (org-set-tags nil 'ignore-column))
+ (when org-auto-align-tags (org-set-tags nil :align-only-current))
(when org-adapt-indentation (org-fixup-indentation diff))
(run-hooks 'org-after-demote-entry-hook))))
@@ -14199,7 +14199,7 @@ ACTION can be `set', `up', `down', or a character."
(insert " [#" news "]"))
(goto-char (match-beginning 3))
(insert "[#" news "] "))))
- (org-set-tags nil 'align))
+ (org-set-tags nil :align-only-current))
(if remove
(message "Priority removed")
(message "Priority of current item set to %s" news)))))
@@ -14955,16 +14955,25 @@ Assume point is on a headline."
;; before tags.
(when (< pos (point)) (goto-char pos))))))
-(defun org-set-tags-command (&optional arg just-align)
- "Call the set-tags command for the current entry."
+(defun org-set-tags-command (&optional just-align align-only-current)
+ "Set the tags for the current headline.
+With prefix JUST-ALIGN, only re-align all tags in headings in the
+current buffer. If JUST-ALIGN is nil and ALIGN-ONLY-CURRENT is
+non-nil, and if the point is on a headline, only align tags in
+that heading.
+
+This command is similar to `org-set-tags', but does not need the
+point to be on an heading when called. "
(interactive "P")
- (if (or (org-at-heading-p) (and arg (org-before-first-heading-p)))
- (org-set-tags arg just-align)
+ (if (or (org-at-heading-p)
+ (and just-align
+ (org-before-first-heading-p)))
+ (org-set-tags just-align align-only-current)
(save-excursion
(unless (and (org-region-active-p)
- org-loop-over-headlines-in-active-region)
- (org-back-to-heading t))
- (org-set-tags arg just-align))))
+ org-loop-over-headlines-in-active-region)
+ (org-back-to-heading t))
+ (org-set-tags just-align align-only-current))))
(defun org-set-tags-to (data)
"Set the tags of the current entry to DATA, replacing the current tags.
@@ -14990,10 +14999,10 @@ If DATA is nil or the empty string, any tags will be removed."
(goto-char (match-beginning 5))
(insert data)
(delete-region (point) (point-at-eol))
- (org-set-tags nil 'align))
+ (org-set-tags nil :align-only-current))
(goto-char (point-at-eol))
(insert " " data)
- (org-set-tags nil 'align)))
+ (org-set-tags nil :align-only-current)))
(beginning-of-line 1)
(when (looking-at ".*?\\([ \t]+\\)$")
(delete-region (match-beginning 1) (match-end 1))))))
@@ -15009,127 +15018,134 @@ If DATA is nil or the empty string, any tags will be removed."
(message "No headings"))))
(defvar org-indent-indentation-per-level)
-(defun org-set-tags (&optional arg just-align)
+(defun org-set-tags (&optional just-align align-only-current)
"Set the tags for the current headline.
-With prefix ARG, realign all tags in headings in the current buffer.
-When JUST-ALIGN is non-nil, only align tags."
+With prefix JUST-ALIGN, only re-align all tags in headings in the
+current buffer. If JUST-ALIGN is nil and ALIGN-ONLY-CURRENT is
+non-nil, and if the point is on a headline, only align tags in
+that heading.
+
+It is assumed that the point is in a heading when this function
+is called."
(interactive "P")
(if (and (org-region-active-p) org-loop-over-headlines-in-active-region)
(let ((cl (if (eq org-loop-over-headlines-in-active-region 'start-level)
'region-start-level
- 'region))
+ 'region))
org-loop-over-headlines-in-active-region)
(org-map-entries
- ;; We don't use ARG and JUST-ALIGN here because these args
- ;; are not useful when looping over headlines.
+ ;; We don't use JUST-ALIGN and ALIGN-ONLY-CURRENT here
+ ;; because these args are not useful when looping over
+ ;; headlines.
#'org-set-tags
org-loop-over-headlines-in-active-region
cl
- '(when (org-invisible-p) (org-end-of-subtree nil t))))
+ '(when (org-invisible-p) (org-end-of-subtree nil t))))
(let ((org-setting-tags t))
- (if arg
+ (if just-align
(save-excursion
(goto-char (point-min))
(while (re-search-forward org-outline-regexp-bol nil t)
- (org-set-tags nil t)
- (end-of-line))
+ (org-set-tags nil :align-only-current)
+ (end-of-line))
(message "All tags realigned to column %d" org-tags-column))
- (let* ((current (org-get-tags-string))
- (tags
- (if just-align current
- ;; Get a new set of tags from the user.
- (save-excursion
- (let* ((seen)
- (table
- (setq
- org-last-tags-completion-table
- ;; Uniquify tags in alists, yet preserve
- ;; structure (i.e., keywords).
- (delq nil
- (mapcar
- (lambda (pair)
- (let ((head (car pair)))
- (cond ((symbolp head) pair)
- ((member head seen) nil)
- (t (push head seen)
- pair))))
- (append
- (or org-current-tag-alist
- (org-get-buffer-tags))
- (and
- org-complete-tags-always-offer-all-agenda-tags
- (org-global-tags-completion-table
- (org-agenda-files))))))))
- (current-tags (org-split-string current ":"))
- (inherited-tags
- (nreverse (nthcdr (length current-tags)
- (nreverse (org-get-tags-at))))))
- (replace-regexp-in-string
- "\\([-+&]+\\|,\\)"
- ":"
- (if (or (eq t org-use-fast-tag-selection)
- (and org-use-fast-tag-selection
- (delq nil (mapcar #'cdr table))))
- (org-fast-tag-selection
- current-tags inherited-tags table
- (and org-fast-tag-selection-include-todo
- org-todo-key-alist))
- (let ((org-add-colon-after-tag-completion
- (< 1 (length table))))
- (org-trim
- (completing-read
- "Tags: "
- #'org-tags-completion-function
- nil nil current 'org-tags-history))))))))))
-
- (when org-tags-sort-function
- (setq tags
- (mapconcat
- #'identity
- (sort (org-split-string tags "[^[:alnum:]_@#%]+")
- org-tags-sort-function)
- ":")))
-
- (if (or (string= ":" tags)
- (string= "::" tags))
- (setq tags ""))
- (if (not (org-string-nw-p tags)) (setq tags "")
- (unless (string-suffix-p ":" tags) (setq tags (concat tags ":")))
- (unless (string-prefix-p ":" tags) (setq tags (concat ":" tags))))
-
- ;; Insert new tags at the correct column.
- (unless (equal current tags)
- (save-excursion
- (beginning-of-line)
- (let ((case-fold-search nil))
- (looking-at org-complex-heading-regexp))
- ;; Remove current tags, if any.
- (when (match-end 5) (replace-match "" nil nil nil 5))
- ;; Insert new tags, if any. Otherwise, remove trailing
- ;; white spaces.
- (end-of-line)
- (if (not (equal tags ""))
- ;; When text is being inserted on an invisible
- ;; region boundary, it can be inadvertently sucked
- ;; into invisibility.
- (outline-flag-region (point) (progn (insert " " tags) (point)) nil)
- (skip-chars-backward " \t")
- (delete-region (point) (line-end-position)))))
- ;; Align tags, if any. Fix tags column if `org-indent-mode'
- ;; is on.
- (unless (equal tags "")
- (let* ((level (save-excursion
- (beginning-of-line)
- (skip-chars-forward "\\*")))
- (offset (if (bound-and-true-p org-indent-mode)
- (* (1- org-indent-indentation-per-level)
- (1- level))
- 0))
- (tags-column
- (+ org-tags-column
- (if (> org-tags-column 0) (- offset) offset))))
- (org--align-tags-here tags-column))))
- (unless just-align (run-hooks 'org-after-tags-change-hook))))))
+ (let* ((current (org-get-tags-string))
+ (tags
+ (if align-only-current
+ current
+ ;; Get a new set of tags from the user.
+ (save-excursion
+ (let* ((seen)
+ (table
+ (setq
+ org-last-tags-completion-table
+ ;; Uniquify tags in alists, yet preserve
+ ;; structure (i.e., keywords).
+ (delq nil
+ (mapcar
+ (lambda (pair)
+ (let ((head (car pair)))
+ (cond ((symbolp head) pair)
+ ((member head seen) nil)
+ (t (push head seen)
+ pair))))
+ (append
+ (or org-current-tag-alist
+ (org-get-buffer-tags))
+ (and
+ org-complete-tags-always-offer-all-agenda-tags
+ (org-global-tags-completion-table
+ (org-agenda-files))))))))
+ (current-tags (org-split-string current ":"))
+ (inherited-tags
+ (nreverse (nthcdr (length current-tags)
+ (nreverse (org-get-tags-at))))))
+ (replace-regexp-in-string
+ "\\([-+&]+\\|,\\)"
+ ":"
+ (if (or (eq t org-use-fast-tag-selection)
+ (and org-use-fast-tag-selection
+ (delq nil (mapcar #'cdr table))))
+ (org-fast-tag-selection
+ current-tags inherited-tags table
+ (and org-fast-tag-selection-include-todo
+ org-todo-key-alist))
+ (let ((org-add-colon-after-tag-completion
+ (< 1 (length table))))
+ (org-trim
+ (completing-read
+ "Tags: "
+ #'org-tags-completion-function
+ nil nil current 'org-tags-history))))))))))
+
+ (when org-tags-sort-function
+ (setq tags
+ (mapconcat
+ #'identity
+ (sort (org-split-string tags "[^[:alnum:]_@#%]+")
+ org-tags-sort-function)
+ ":")))
+
+ (if (or (string= ":" tags)
+ (string= "::" tags))
+ (setq tags ""))
+ (if (not (org-string-nw-p tags)) (setq tags "")
+ (unless (string-suffix-p ":" tags) (setq tags (concat tags ":")))
+ (unless (string-prefix-p ":" tags) (setq tags (concat ":" tags))))
+
+ ;; Insert new tags at the correct column.
+ (unless (equal current tags)
+ (save-excursion
+ (beginning-of-line)
+ (let ((case-fold-search nil))
+ (looking-at org-complex-heading-regexp))
+ ;; Remove current tags, if any.
+ (when (match-end 5) (replace-match "" nil nil nil 5))
+ ;; Insert new tags, if any. Otherwise, remove trailing
+ ;; white spaces.
+ (end-of-line)
+ (if (not (equal tags ""))
+ ;; When text is being inserted on an invisible
+ ;; region boundary, it can be inadvertently sucked
+ ;; into invisibility.
+ (outline-flag-region (point) (progn (insert " " tags) (point)) nil)
+ (skip-chars-backward " \t")
+ (delete-region (point) (line-end-position)))))
+ ;; Align tags, if any. Fix tags column if `org-indent-mode'
+ ;; is on.
+ (unless (equal tags "")
+ (let* ((level (save-excursion
+ (beginning-of-line)
+ (skip-chars-forward "\\*")))
+ (offset (if (bound-and-true-p org-indent-mode)
+ (* (1- org-indent-indentation-per-level)
+ (1- level))
+ 0))
+ (tags-column
+ (+ org-tags-column
+ (if (> org-tags-column 0) (- offset) offset))))
+ (org--align-tags-here tags-column))))
+ (unless align-only-current (run-hooks 'org-after-tags-change-hook))))))
(defun org-change-tag-in-region (beg end tag off)
"Add or remove TAG for each entry in the region.
@@ -16164,10 +16180,10 @@ decreases scheduled or deadline date by one day."
((not (member value org-todo-keywords-1))
(user-error "\"%s\" is not a valid TODO state" value)))
(org-todo value)
- (org-set-tags nil 'align))
+ (org-set-tags nil :align-only-current))
((equal property "PRIORITY")
(org-priority (if (org-string-nw-p value) (string-to-char value) ?\s))
- (org-set-tags nil 'align))
+ (org-set-tags nil :align-only-current))
((equal property "SCHEDULED")
(forward-line)
(if (and (looking-at-p org-planning-line-re)
@@ -20254,7 +20270,7 @@ The detailed reaction depends on the user option `org-catch-invisible-edits'."
(defun org-fix-tags-on-the-fly ()
"Align tags in headline at point.
-Unlike to `org-set-tags', it ignores region and sorting."
+Unlike `org-set-tags', it ignores region and sorting."
(when (and (eq (char-after (line-beginning-position)) ?*) ;short-circuit
(org-at-heading-p))
(let ((org-ignore-region t)
--
2.13.0
next reply other threads:[~2017-07-12 2:47 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-12 2:46 Kaushal Modi [this message]
2017-07-13 7:54 ` [PATCH] Refactor org-set-tags arguments for clarity Nicolas Goaziou
2017-07-13 10:18 ` Kaushal Modi
2017-07-13 11:58 ` Nicolas Goaziou
2017-07-13 12:21 ` Kaushal Modi
2017-07-13 12:31 ` Nicolas Goaziou
2017-07-13 12:37 ` Kaushal Modi
2017-07-13 12:39 ` Kaushal Modi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFyQvY1aDNQgnwwtWaC00Z__okrDT_Y0RLbPko-_=tNc+pNZjA@mail.gmail.com' \
--to=kaushal.modi@gmail.com \
--cc=emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.