From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: John Kitchin <jkitchin@andrew.cmu.edu>
Cc: "emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: Re: Draft of links-9.0
Date: Tue, 05 Jul 2016 23:56:29 +0200 [thread overview]
Message-ID: <87twg3hhpu.fsf@saiph.selenimh> (raw)
In-Reply-To: <m2wpkzesjm.fsf@Johns-MacBook-Air.local> (John Kitchin's message of "Tue, 05 Jul 2016 16:30:53 -0400")
John Kitchin <jkitchin@andrew.cmu.edu> writes:
> I pasted the diffs at the end. There are a lot because it touched a lot of
> files, and I committed each one separately, and forgot a few commits
> that ended up as extras. Probably some could be combined and squashed if
> that is desirable. I am not sure how to do it yet though.
Clearly some of them could be merged. From magit, this is "r i" from the
first commit (aka interactive rebase) then, you can squash or fixup
commits, reorder them...
Comments follow, in reverse order.
> commit 546031bb7b59a9deb6bce976f7f7496ea7b2ffc4
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 16:16:29 2016 -0400
>
> add link-start and link-end variables
This can probably be merged with another commit.
> commit d1799252a7e4bd255a5507d742e0b0a7c19983b1
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 16:16:02 2016 -0400
>
> deprecate org-add-link-type
>
> It now calls org-link-add.
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 4fb1cfb..872f861 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -9852,10 +9852,10 @@ Special properties are:
> In addition to these, any additional properties can be specified
> and then used in capture templates.")
>
> -(cl-defun org-add-link-type (type &optional follow export
> - &key store complete face mouse-face display
> - help-echo keymap htmlize-link activate-func)
> +(defun org-add-link-type (type &optional follow export)
> "Add a new TYPE link.
> +Deprecated. Use `org-link-add' instead.
No need to add this in the docstring. Use `make-obsolete'. Also,
obsolete definitions should be moved to "org-compat.el".
> + (unless (cdr (assoc type org-link-parameters))
> + (error "No link of type %s defined." type))
> + (cl-loop for (key val) on parameters by #'cddr
> + do
> + (setf (cl-getf
> + (cdr (assoc type org-link-parameters))
> + key)
> + val))
> org-link-parameters)
See below for a suggested implementation of the function.
> +(defun org-link-add (type &rest properties)
> + "Add a TYPE link with PROPERTIES defined as a plist.
> +See `org-link-parameters' for details on the PROPERTIES."
> + (push (append (list type) properties)
> + org-link-parameters)
> + (org-make-link-regexps)
> + (org-element-update-syntax))
This function is not needed since it is the same as
`org-link-set-parameters'.
> (defun org-link-types ()
> "Returns a list of known link types."
> (mapcar 'car org-link-parameters))
>
> commit 1069d9bac0974f27abda2736fef883eb8334c761
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 16:14:02 2016 -0400
>
> update comment for new variable.
"Update ..."
This can probably be merged with another commit.
I skipped intermediary commits. You can merge them.
> commit a883c04c1c9cd9ae7eeaac485eebebc49204f5d3
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:42:49 2016 -0400
>
> remove copy-sequence
>
> Actually it seems like the let statement is no longer needed. I guess it
> was around to protect the list from extra links getting defined.
You can merge this commit with another one below.
> commit c922ab8b2d19d265f8183a5b0d36adb9b61087ce
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:30:48 2016 -0400
>
> replace an org-link-types variable with function
You can merge it with a commit below.
> commit 75fa053296d8b4bbb8e7edeb9b7e077eb8ed37f1
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:28:38 2016 -0400
>
> update w3m link
>
> commit c41dcddfa8ba848d4e911282bc5760a4e4b953ff
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:28:09 2016 -0400
>
> update rmail link definition
>
> commit 86221a71daa897d94cb8ca7aa4f63f0b4e897477
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:27:55 2016 -0400
>
> update mhe link definition.
>
> commit fc6accee74dd61dc58cab3e5d70fbf411faedb74
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:27:45 2016 -0400
>
> update irc link definition
>
> commit 6e7d024b316fb8c9f1ebf37421a50917ac69aa15
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:27:32 2016 -0400
>
> update info link definition
>
> commit 898abadf1decbf730a679ba34f5b26a37f52bcf5
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:27:14 2016 -0400
>
> update gnus link definition
>
> commit 917114bd4968f7ff8e653950af194a70d625c849
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:27:03 2016 -0400
>
> update eshell link definition
>
> commit 626682724311ca54292a34f2b9a4b8aa519c6fe8
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:26:28 2016 -0400
>
> Update docview link definition
>
> commit 2f32ebf222c6d1e391358d6146ebc6f21a4bf095
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:26:04 2016 -0400
>
> update bibtex link definition.
>
> * lisp/org-bibtex.el ("bibtex"):
>
> commit 776f20bfc5afd744c6318f9d66ced055c02a88c5
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:25:39 2016 -0400
>
> update bbdb link definition.
>
> * lisp/org-bbdb.el ("bbdb"):
Skipped type specific patches. Please mind the commit messages, though.
> commit 5a296333a30e3880e5f551eb82c8b0219447e793
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:24:01 2016 -0400
>
> add activate-func
"Add activate-func"
> * lisp/org.el (org-activate-bracket-links):
> This should have been in the previous commit on this function.
You can merge commits easily.
> commit c306e89aa41b8f84107dc60957540e3eaa85d009
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:23:27 2016 -0400
>
> replace org-link-protocols with the link export parameter.
Commit message is incomplete. Otherwise, OK.
> commit 50d93f950f68587bad239b8fa26741f8b44675d8
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:22:24 2016 -0400
>
> Make plain and bracketed link properties stick
>
> * lisp/org.el (org-set-font-lock-defaults): If t is after the face, than
> org-link clobbers everything from the activation functions.
Ok.
> commit 70cd41305abdce2f5deb458cea6b493d6c850e3c
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:21:50 2016 -0400
>
> Update org-activate-bracket-links
`org-activate-bracket-links'
> * lisp/org.el (org-activate-bracket-links): Use org-link-parameters to
> build link properties.
`org-link-parameters'
> + ;; An anonymous face
> + ((listp link-face)
> + link-face)
See below.
> + ;; default
> + (t
> + 'org-link))
> + 'keymap (or (org-link-get-parameter type :keymap)
> + org-mouse-map)
> + 'mouse-face (or (org-link-get-parameter type :mouse-face)
> + 'highlight)
> + 'font-lock-multiline t
> + 'help-echo help
> + 'htmlize-link (cond
> + ((functionp htmlize-link)
> + (funcall htmlize-link))
> + (t
> + `(:uri ,(format "%s:%s" type path))))))
> + ;; visible part
> + (vp (list 'keymap (or (org-link-get-parameter type :keymap)
> + org-mouse-map)
> + 'face (cond
> + ;; A function that returns a face
> + ((functionp link-face)
> + (funcall link-face path))
> + ;; a face
> + ((facep link-face)
> + link-face)
> + ;; An anonymous face
> + ((listp link-face)
> + link-face)
> + ;; default
> + (t
> + 'org-link))
> + 'mouse-face (or (org-link-get-parameter type :mouse-face)
> + 'highlight)
> + 'font-lock-multiline t
> + 'help-echo help
> + 'htmlize-link (cond
> + ((functionp htmlize-link)
> + (funcall htmlize-link))
> + (t
> + `(:uri ,(format "%s:%s" type path)))))))
There is some code duplication going on here. It doesn't matter much for
now.
> commit d2d39fc4ea24e17dd24c0bc78b4ef1cc14ef8258
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:21:08 2016 -0400
>
> update org-activate-plain-links
`org-activate-plain-links'
>
> * lisp/org.el (org-activate-plain-links): Use org-link-parameters to
> create the link properties.
`org-link-parameters'
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 6dfd089..21122b7 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -5917,17 +5917,55 @@ prompted for."
> "Add link properties for plain links."
> (when (and (re-search-forward org-plain-link-re limit t)
> (not (org-in-src-block-p)))
> - (let ((face (get-text-property (max (1- (match-beginning 0)) (point-min))
> - 'face))
> - (link (match-string-no-properties 0)))
> +
> + (let* ((face (get-text-property (max (1- (match-beginning 0)) (point-min))
> + 'face))
> + (link (match-string-no-properties 0))
> + (type (match-string-no-properties 1))
> + (path (match-string-no-properties 2))
> + (link-face (org-link-get-parameter type :face))
> + (help-echo (org-link-get-parameter type :help-echo))
> + (htmlize-link (org-link-get-parameter type :htmlize-link))
> + (activate-func (org-link-get-parameter type :activate-func)))
> (unless (if (consp face) (memq 'org-tag face) (eq 'org-tag face))
> (org-remove-flyspell-overlays-in (match-beginning 0) (match-end 0))
> (add-text-properties (match-beginning 0) (match-end 0)
> - (list 'mouse-face 'highlight
> - 'face 'org-link
> - 'htmlize-link `(:uri ,link)
> - 'keymap org-mouse-map))
> + (list
I suggest to move `list' under `add-text-properties'
> + 'mouse-face (or (org-link-get-parameter type :mouse-face)
> + 'highlight)
> + 'face (cond
> + ;; A function that returns a face
> + ((functionp link-face)
> + (funcall link-face path))
> + ;; a face
> + ((facep link-face)
> + link-face)
> + ;; An anonymous face
> + ((listp link-face)
> + link-face)
This should be (consp link-face) so as to avoid nil face, shouldn't it?
> commit 66ad0a6d2c79ac0424f8424b875d707e36fe11d2
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:20:23 2016 -0400
>
> get complete function from org-link-parameters.
Commit message is incomplete.
> (defun org-link-try-special-completion (type)
> "If there is completion support for link type TYPE, offer it."
> - (let ((fun (intern (concat "org-" type "-complete-link"))))
> + (let ((fun (or (org-link-get-parameter type :complete)
> + ;; We still honor the "org-type-complete-link
> + ;; form. There are some built in links like
> + ;; org-file-complete-link that don't fit nicely into
> + ;; org-link-parameters.
> + (intern (concat "org-" type "-complete-link")))))
Eww. How so? Why couldn't `org-file-complete-link' the value
of :complete property of "file" type in `org-link-parameters'?
> commit 251b574b8669d3ed685542eec03c89336c0c91c5
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:19:52 2016 -0400
>
> Replace org-store-link-functions variable with function
You need to specify what function is modified in the commit message.
> diff --git a/lisp/org.el b/lisp/org.el
> index 59d6c63..f9e5487 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -9857,7 +9857,7 @@ active region."
> (delq
> nil (mapcar (lambda (f)
> (let (fs) (if (funcall f) (push f fs))))
> - org-store-link-functions))
> + (org-store-link-functions)))
> sfunsn (mapcar (lambda (fu) (symbol-name (car fu))) sfuns))
> (or (and (cdr sfuns)
> (funcall (intern
>
> commit c8580dd27d0f71649a938c56357b471f58b2aa97
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:18:57 2016 -0400
>
> Update org-add-link-type
>
> * lisp/org.el org-add-link-type: Now accepts keyword arguments for link parameters.
This one can be removed.
> commit 763a0a8cb636bccf6a0211447d287296cd624ccb
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:16:38 2016 -0400
>
> Remove org-link-protocols
Missing:
lisp/org.el (org-link-protocols): Remove variable.
Looks good otherwise.
> commit 91d90b6bef8fac0a94efdf915561a03e75526b11
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:13:17 2016 -0400
>
> Replace org-link-types with function
>
> * lisp/org.el: The variable was replaced with the function
> everywhere.
Looks good, but it could be merged with the commit actually defining the function.
> commit 31f6309dd86344b03f3e58263d05ee2ec688c995
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:08:14 2016 -0400
>
> Add org-link-parameters
>
> * lisp/org.el (org-link-parameters): Add new centralized link variable.
>
> (org-link-get-parameter): A getter function to get link parameters
>
> (org-link-set-parameters): A setter function to set link parameters
>
> (org-link-types): A function to return a list of link types.
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 6dd7f91..8300f8f 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -1758,6 +1758,62 @@ calls `table-recognize-table'."
> "Buffer-local version of `org-link-abbrev-alist', which see.
> The value of this is taken from the #+LINK lines.")
>
> +(defcustom org-link-parameters
> + '(("http") ("https") ("ftp") ("mailto")
> + ("file") ("file+emacs") ("file+sys")
> + ("news") ("shell") ("elisp")
> + ("doi") ("message") ("help"))
I think follow function could be extracted from `org-open-at-point' for,
at least, "help", "doi", "message", "shell" and "elisp". This is not
blocking anyway.
> + "An alist of properties that defines all the links in org-mode.
org-mode -> Org mode.
> +The first key in each list is a string of the link type.
The key in each association is ...
> +Subsequent optional elements make up a p-list of link properties.
> + :follow - a function that takes the link path as an argument
> + :export - a function that takes the link path, description and
> + export-backend as arguments
> + :store - a function responsible for storing the link. See the variable `org-store-link-functions'.
> + :complete - a function that inserts a link with completion. The function takes one optional
> + prefix arg.
> + :face - a face for the link, or a function that returns a face. The function takes one argument which is the link path. The default face is `org-link'.
> + :mouse-face - The mouse-face. The default is `highlight'.
> + :display - `full' will not fold the link in descriptive display. Default is `org-link'.
> + :help-echo - A string or function that takes (window object position) as args and
> + returns a string.
> + :keymap - A keymap that is active on the link. The default is `org-mouse-map'.
> + :htmlize-link - a function for the htmlize-link. Defaults to (list :uri \"type:path\")
> + :activate-func - a function to run at the end of font-lock activation. The function must accept
> +(link-start link-end path bracketp) as arguments."
You need to separate sentences with two spaces. Also, lines should be
shorter.
> + :group 'org-link
> + :type '(alist :tag "Link display paramters"
> + :type string
> + :follow :export :store :completion :face :mouse-face
> + :display :help-echo :keymap :htmlize-link :activate-func))
I think this :type is invalid.
:type '(alist :tag "Link display parameters"
:value-type (plist))
is a good start, but ultimately, it should be more subtle.
> +(defun org-link-get-parameter (type key)
> + "Get TYPE link property for KEY."
> + (cl-getf
> + (cdr (assoc type org-link-parameters))
> + key))
Nitpick: `cl-getf' -> `plist-get'
> +(defun org-link-set-parameters (type &rest parameters)
> + "Set link TYPE properties to PARAMETERS.
> +PARAMETERS should be :key val pairs."
> + (if (cdr (assoc type org-link-parameters))
> + ;; update old link
> + (cl-loop for (key val) on parameters by #'cddr
> + do
> + (setf (cl-getf
> + (cdr (assoc type org-link-parameters))
> + key)
> + val))
> + ;; add new one.
> + (push `(,type ,@parameters) org-link-parameters)
> + (org-make-link-regexps)
> + (org-element-update-syntax))
> + org-link-parameters)
There's already `org-combine-plists'. Also I don't think returning
`org-link-parameters' is needed. If you think it is, it should be
notified in the docstring.
(defun org-link-set-parameters (type &rest parameters)
"Set link TYPE properties to PARAMETERS.
PARAMETERS should be :key val pairs."
(let ((data (assoc type org-link-parameters)))
(if data (setcdr (cdr data) (org-combine-plists (cdr data) parameters))
(push (cons type parameters) org-link-parameters)
(org-make-link-regexps)
(org-element-update-syntax))))
> +(defun org-link-types ()
> + "Returns a list of known link types."
> + (mapcar 'car org-link-parameters))
Nitpick (mapcar #'car ...)
> commit 8b8f65f4314fa9eb08bc5bd7c23dcf32f456f57d
> Author: John Kitchin <jkitchin@andrew.cmu.edu>
> Date: Tue Jul 5 10:07:10 2016 -0400
>
> Remove org-link-types
>
> * lisp/org.el (org-link-types): Remove this variable.
This one can be merged above.
Regards,
next prev parent reply other threads:[~2016-07-05 21:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-05 14:50 Draft of links-9.0 John Kitchin
2016-07-05 19:12 ` Nicolas Goaziou
2016-07-05 19:42 ` Nicolas Goaziou
2016-07-05 20:30 ` John Kitchin
2016-07-05 21:56 ` Nicolas Goaziou [this message]
2016-07-06 1:49 ` John Kitchin
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
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87twg3hhpu.fsf@saiph.selenimh \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=jkitchin@andrew.cmu.edu \
/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 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).