[PATCH] Refactor org-set-tags arguments for clarity

[PATCH] Refactor org-set-tags arguments for clarity

[Kaushal Modi]

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

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Nicolas Goaziou]

Hello,

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

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

Thank you. LGTM!

Actually, I think ALIGN-ONLY-CURRENT could be merged with JUST-ALIGN,
which would have three cases (e.g., t, nil, `current'). Perhaps the
incompatibility it introduces is not acceptable though.

While you're at it, what about throwing in some test in
"test-org/set-tags"? ;)

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Kaushal Modi]

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

On Thu, Jul 13, 2017, 3:54 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Thank you. LGTM!
>

Thanks.

Actually, I think ALIGN-ONLY-CURRENT could be merged with JUST-ALIGN,
> which would have three cases (e.g., t, nil, `current'). Perhaps

 the
> incompatibility it introduces is not acceptable though.
>

I was itching to do that. But there are dozens of (org-set-tags nil t)
instances in the Org source itself. I though just created a wrapper in my
personal config to do ALIGN-ONLY-CURRENT when prefix is C-u C-u.

I can do the same in the source too, while keeping the argument order as it
is. I can use current-prefix-arg to override the value of JUST-ALIGN to nil
and ALIGN-ONLY-CURRENT to t internally for interactive use. The elisp use
of this function will stay unchanged. I'll apply this piece just to the
master if you like.

While you're at it, what about throwing in some test in
> "test-org/set-tags"? ;)
>

Will do.

PS: Also, in addition, was thinking of calling JUST-ALIGN ALIGN-ALL
instead.

With:

    (defun org-set-tags (&optional ALIGN-ALL ALIGN-ONLY-CURRENT) ..

it will be more apparent that they are mutually exclusive args.

WDYT?
-- 

Kaushal Modi

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

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Nicolas Goaziou]

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

> I was itching to do that. But there are dozens of (org-set-tags nil t)

Calls in code base do not matter, since we can change them. The above
Sexp would be equivalent to 

  (org-set-tags 'current)

> instances in the Org source itself. I though just created a wrapper in my
> personal config to do ALIGN-ONLY-CURRENT when prefix is C-u C-u.

I don't think an user needs to distinguish between aligning all and
aligning only current. I think this is just confusing.

> PS: Also, in addition, was thinking of calling JUST-ALIGN ALIGN-ALL
> instead.

Sounds good.

> With:
>
>     (defun org-set-tags (&optional ALIGN-ALL ALIGN-ONLY-CURRENT) ..
>
> it will be more apparent that they are mutually exclusive args.

It still bugs me because (org-set-tags t t) doesn't make any sense.

WDYT?

Regards,

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Kaushal Modi]

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

On Thu, Jul 13, 2017 at 7:58 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

>
> Calls in code base do not matter, since we can change them. The above
> Sexp would be equivalent to
>
>   (org-set-tags 'current)
>

Correct. I was just extrapolating based on that, that people could be
making similar uses in their configs and packages.

A search like this (
https://github.com/search?utf8=%E2%9C%93&q=%22org-set-tags+nil%22+language%3A%22Emacs+Lisp%22&type=Code
)
shows that (org-set-tags nil t) is pretty viral in one org-settings.el out
there.


> > instances in the Org source itself. I though just created a wrapper in my
> > personal config to do ALIGN-ONLY-CURRENT when prefix is C-u C-u.
>
> I don't think an user needs to distinguish between aligning all and
> aligning only current. I think this is just confusing.
>

That's alright. I can see that it might not be that useful as org-set-tags
anyways aligns the tags. (I am just used to abusing the free C-u C-u in my
config :))


> > PS: Also, in addition, was thinking of calling JUST-ALIGN ALIGN-ALL
> > instead.
>
> Sounds good.
>

Thanks.


> > With:
> >
> >     (defun org-set-tags (&optional ALIGN-ALL ALIGN-ONLY-CURRENT) ..
> >
> > it will be more apparent that they are mutually exclusive args.
>
> It still bugs me because (org-set-tags t t) doesn't make any sense.
>

Same sentiments here (I even fixed one case of (org-set-tags t t) in that
patch).


> WDYT?
>

The only concern is the one I presented above; doing this will break many
personal configs.
-- 

Kaushal Modi

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

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Nicolas Goaziou]

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

> Correct. I was just extrapolating based on that, that people could be
> making similar uses in their configs and packages.
>
> A search like this (
> https://github.com/search?utf8=%E2%9C%93&q=%22org-set-tags+nil%22+language%3A%22Emacs+Lisp%22&type=Code
> )
> shows that (org-set-tags nil t) is pretty viral in one org-settings.el out
> there.

Then what about something like this

  (defun org-set-tags (&optional align-all align-current)
    "...
  ...
  ALIGN-CURRENT is obsolete and should not be used. When non-nil,
  set ALIGN-ALL to `current'."
    (let ((align-all (if (null align-current) align-all
                       (warn "Deprecated call to `org-set-tags', which see")
                       'current)))
     ...))


Regards,

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Kaushal Modi]

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

On Thu, Jul 13, 2017 at 8:32 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> > Correct. I was just extrapolating based on that, that people could be
> > making similar uses in their configs and packages.
> >
> > A search like this (
> >
> https://github.com/search?utf8=%E2%9C%93&q=%22org-set-tags+nil%22+language%3A%22Emacs+Lisp%22&type=Code
> > )
> > shows that (org-set-tags nil t) is pretty viral in one org-settings.el
> out
> > there.
>
> Then what about something like this
>
>   (defun org-set-tags (&optional align-all align-current)
>     "...
>   ...
>   ALIGN-CURRENT is obsolete and should not be used. When non-nil,
>   set ALIGN-ALL to `current'."
>     (let ((align-all (if (null align-current) align-all
>                        (warn "Deprecated call to `org-set-tags', which
> see")
>                        'current)))
>      ...))
>

That will work. Good idea!

I'll first patch maint, and then this in master.
-- 

Kaushal Modi

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

Re: [PATCH] Refactor org-set-tags arguments for clarity

[Kaushal Modi]

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

On Thu, Jul 13, 2017 at 8:37 AM Kaushal Modi <kaushal.modi@gmail.com> wrote:

>   (defun org-set-tags (&optional align-all align-current)
>>     "...
>>   ...
>>   ALIGN-CURRENT is obsolete and should not be used. When non-nil,
>>   set ALIGN-ALL to `current'."
>>     (let ((align-all (if (null align-current) align-all
>>                        (warn "Deprecated call to `org-set-tags', which
>> see")
>>                        'current)))
>>      ...))
>>
>
> That will work. Good idea!
>
> I'll first patch maint, and then this in master.
>

And then.. ALIGN-ALL can become JUST-ALIGN in master :P
-- 

Kaushal Modi

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