[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] New "project" option for org-link-file-path-type
From: |
Kyle Meyer |
Subject: |
Re: [PATCH] New "project" option for org-link-file-path-type |
Date: |
Mon, 02 Nov 2020 00:41:49 -0500 |
Jack Kamm writes:
> The attached patch adds a "project" option for org-link-file-path-type.
>
> When this is set, links to files under the current project root will be
> relative, while links elsewhere are absolute.
Thanks, that sounds useful.
> It relies on project.el, which appears to have been added in emacs 25. I
> used fboundp to check whether the functionality is available, and to
> silence compiler warnings. I'm not sure if this is the correct way to do
> it.
Functionally I think your current patch would only support Emacs's
unreleased master, unless the user installed a new project.el via ELPA.
More on that below.
> Subject: [PATCH] ol.el: New option "project" for org-link-file-path-type
[...]
> @@ -212,13 +212,17 @@ (defcustom org-link-file-path-type 'adaptive
> absolute Absolute path, if possible with ~ for home directory.
> noabbrev Absolute path, no abbreviation of home directory.
> adaptive Use relative path for files in the current directory and sub-
> - directories of it. For other files, use an absolute path."
> + directories of it. For other files, use an absolute path.
> +project Use relative path for files in the current project and sub-
> + directories of it. For other files, usue an absolute path.
s/usue/use/
> + If project.el is not available, behave as adaptive."
> :group 'org-link
> :type '(choice
> (const relative)
> (const absolute)
> (const noabbrev)
> - (const adaptive))
> + (const adaptive)
> + (const project))
> :safe #'symbolp)
The :package-version keyword should be added to signal the change in
value.
> (defcustom org-link-abbrev-alist nil
> @@ -1876,6 +1880,15 @@ (defun org-insert-link (&optional complete-file
> link-location description)
> (setq path (expand-file-name path)))
> ((eq org-link-file-path-type 'relative)
> (setq path (file-relative-name path)))
> + ((and (fboundp 'project-current)
> + (fboundp 'project-root)
> + (project-current)
> + (eq org-link-file-path-type 'project))
Minor: the org-link-file-path-type check would be better positioned at
the top, or at least before the project-current call, to avoid
needlessly finding the project when the option is at its default value
of adaptive.
Also, I think it'd be good to let-bind the project-current result to
avoid a repeated call.
> + (if (string-prefix-p (expand-file-name (project-root
> + (project-current)))
> + (expand-file-name path))
It looks like project-root isn't available until 5044c19001 (project.el:
A project has only one main root now, 2020-05-23), which isn't yet part
of an Emacs release. Before that, it'd be (car (project-roots ...), I
think. Do you think it's worth adding a compatibility kludge here?
As a projectile user, I'm tempted to suggest that, instead of the adding
the `project' value, org-insert-link could learn to call
org-link-file-path-type if it is a function and, if that returns
non-nil, do the prefix check. Then projectile users could set it to
projectile-project-root. It seems project.el doesn't have a similar
function that could be called without any arguments, but I guess we
could add a simple ol- wrapper. I'm not sure that's a good idea,
though.
> + (setq path (file-relative-name path))
> + (setq path (abbreviate-file-name (expand-file-name path)))))
A cosmetic preference that you can take or leave: the condition can be
moved inside the setq form:
(setq path (if ...))
And another: let-binding (expand-file-name path) would avoid a repeating
the expand-file-name in the abbreviate-file-name case.
- Re: [PATCH] New "project" option for org-link-file-path-type,
Kyle Meyer <=