diff options
| author | Jonas Bernoulli <jonas@bernoul.li> | 2025-08-22 20:55:39 +0200 |
|---|---|---|
| committer | Jonas Bernoulli <jonas@bernoul.li> | 2025-08-22 20:55:39 +0200 |
| commit | 67a662022eb5e00ad0c281255c308ab5fce28985 (patch) | |
| tree | daaaf79911f4e18e8b9f1effa4f13d483db99a6f /lisp/magit-push.el | |
| parent | 1c48327a067c41f78f35cc65bdbf3067f1d3d25d (diff) | |
Avoid t condition in final match-all cond clause
If there is a single BODY form in the final clause, we can just
use that as the CONDITION instead (because the value of that is
returned if the BODY is empty).
I've been gravitating towards that style for a long time, but
because I realize that this is not without issue and that there
are certainly those that would find this practice questionable,
I refrained from updating existing code until now.
The main problem with dropping the `t' is that it is not always
immediately obvious if one is looking at
(cond
...
((foo bar)))
or
(cond
...
((foo) (bar)))
That's why I still use `t' when the single BODY form is too long
and/or complex, despite fitting on one line. In this case from
`magit-log-move-to-revision', for example, I was on the fence:
(cond
...
((apply #'magit-log-all-branches (magit-log-arguments))))
(It's worth noting that the same issue also occurs when reading
non-final-catch-all clauses.)
On the other hand, the single BODY form can also be complex
*enough* to allow using it as the CONDITION, without that being hard
to parse. E.g., when it spans multiple lines, and indentation keeps
increasing, and/or when BODY is a `let' from or similar.
Used like this, `cond' becomes more similar to `or' in certain
situations, but even then they still behave differently, consider:
(or (and CONDITION maybe-nil)
...)
vs
(cond (CONDITION maybe-nil)
...)
Diffstat (limited to 'lisp/magit-push.el')
| -rw-r--r-- | lisp/magit-push.el | 6 |
1 files changed, 2 insertions, 4 deletions
diff --git a/lisp/magit-push.el b/lisp/magit-push.el index c64acb5..f603250 100644 --- a/lisp/magit-push.el +++ b/lisp/magit-push.el @@ -107,8 +107,7 @@ argument the push-remote can be changed before pushed to it." 'magit-branch-remote))) (remote (format "%s, replacing invalid" v)) - (t - (format "%s, setting that" v))))) + ((format "%s, setting that" v))))) ;;;###autoload (autoload 'magit-push-current-to-upstream "magit-push" nil t) (transient-define-suffix magit-push-current-to-upstream (args) @@ -171,8 +170,7 @@ the upstream." (magit--propertize-face merge 'magit-branch-remote))) ((or remote merge) (concat u ", creating it and replacing invalid")) - (t - (concat u ", creating it"))))))) + ((concat u ", creating it"))))))) ;;;###autoload (defun magit-push-current (target args) |
