emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Jack Kamm <jackkamm@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] New "project" option for org-link-file-path-type
Date: Mon, 02 Nov 2020 00:41:49 -0500	[thread overview]
Message-ID: <874km89uxu.fsf@kyleam.com> (raw)
In-Reply-To: <87v9etq2pg.fsf@gmail.com>

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.


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

>  (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,

> +		(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.

  reply	other threads:[~2020-11-02  5:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-29  0:46 [PATCH] New "project" option for org-link-file-path-type Jack Kamm
2020-11-02  5:41 ` Kyle Meyer [this message]
2020-11-04 18:11   ` Jack Kamm
2020-11-06  3:33     ` Kyle Meyer
2020-11-12  1:05       ` Jack Kamm
2021-04-25  3:29         ` Timothy

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:

  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=874km89uxu.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=jackkamm@gmail.com \


* 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


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).