From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Rasmus <rasmus@gmx.us>
Cc: emacs-orgmode@gnu.org
Subject: Re: [ox-publish, patch] More flexible sitemaps
Date: Wed, 01 Jun 2016 17:34:56 +0200 [thread overview]
Message-ID: <87wpm8j527.fsf@saiph.selenimh> (raw)
In-Reply-To: <87h9djv4gw.fsf@gmx.us> (rasmus@gmx.us's message of "Fri, 27 May 2016 18:41:03 +0200")
Hello,
Rasmus <rasmus@gmx.us> writes:
> This was by far the hardest part...
Thank you. Some comments follow.
> +(defun org-publish-find-property (file property &optional reset)
> + "Find the PROPERTY of FILE in project.
> +PROPERTY can be a string or a symbol-property."
Could you also document RESET argument?
> + (unless (or (file-directory-p file) (directory-name-p file))
What is `directory-name-p'?
> + (let ((prop (cond ((stringp property)
> + (intern (downcase (format ":%s" property))))
> + (t property))))
The following might be more robust:
(if (keywordp property) property
(intern (downcase (format ":%s" property)))
> + (and (not reset) (org-publish-cache-get-file-property file prop nil t))
(unless reset ...)
Anyway, you don't seem to use the return value. If this is used for
side-effect only, you may want to call `org-publish-initialize-cache'
instead.
> + (let* ((org-inhibit-startup t)
> + (visiting (find-buffer-visiting file))
> + (buffer (or visiting (find-file-noselect file)))
> + (case-fold-search t))
> + (with-current-buffer buffer
> + ;; protect local variables in open buffers
> + (org-export-with-buffer-copy
> + (let* ((backends (append (list nil)
> + (mapcar 'org-export-backend-name
> + org-export-registered-backends)))
> + (value (cl-loop for backend in backends
> + when (org-no-properties
> + (org-element-interpret-data
> + (plist-get (org-export-get-environment backend)
> + prop)))
> + return it)))
Return value of `org-element-interpret-data' is never nil so the loop
always returns the first back-end.
In any case, this is sub-optimal since common keywords are retrieved for
each back-end. Also, two back-ends could use the same keyword, with
different values (e.g., using `parsed' or not).
One option could be to change the policy about keywords in ox.el and
move KEYWORDS and SUBTITLE there. Unfortunately, there is still a change
to miss some cases.
Another option would be to provide BACKEND to sitemap-function. I think
it can be backward-compatible if we make it an optional argument.
> +(defcustom org-publish-sitemap-file-entry-format "%- [[file:%link][%TITLE]]"
> "Format string for site-map file entry.
> -You could use brackets to delimit on what part the link will be.
>
> -%t is the title.
> -%a is the author.
> -%d is the date formatted using `org-publish-sitemap-date-format'."
> +Most keywords can be accessed using a the name of the keyword
> +prefixed with \"%\", e.g. \"%TITLE\" or \"%KEYWORDS\". In
> +addition, the following keywords are defined.
> +
> + %fullpath
> + The full path of the file.
> +
> + %relpath
> + The relative path of the file.
> +
> + %filename
> + %f
> + The filename.
> +
> + %link
> + %l
> + The link.
> +
> + %*
> + Leveled headline relative to the base directory.
> +
> + %*
> + Leveled item relative to the base directory.
> +
> + %author
> + The author of the document, the project, or `user-full-name'.
> +
> + %date
> + Date formatted using `org-publish-sitemap-date-format'.
> +
> +See also `org-publish-sitemap-dir-entry-format'."
Could it be possible to use single-char placeholders and `format-spec',
which is more robust than replace-match (e.g., it is possibl to escape
percent signs)...
> +(defcustom org-publish-sitemap-dir-entry-format "%- %f"
> + "Format string for site-map file entry.
> +
> +The following keywords are defined.
> +
> + %fullpath
> + The full path of the file.
> +
> + %relpath
> + The relative path of the file.
> +
> + %filename
> + %f
> + The filename.
> +
> + %link
> + %l
> + The link.
> +
> + %*
> + Leveled headline relative to the base directory.
> +
> + %*
> + Leveled item relative to the base directory.
> +
> + %author
> + The author of the document, the project, or `user-full-name'.
> +
> + %date
> + Date formatted using `org-publish-sitemap-date-format'.
Ditto.
> @@ -1292,11 +1394,11 @@ Returns value on success, else nil."
> (defun org-publish-cache-ctime-of-src (file)
> "Get the ctime of FILE as an integer."
> (let ((attr (file-attributes
> - (expand-file-name (or (file-symlink-p file) file)
> - (file-name-directory file)))))
> + (expand-file-name (or (file-symlink-p file) file)
> + (file-name-directory file)))))
Maybe
(file-truename file)
instead.
> + `:sitemap-preamble'
> + `:sitemap-postamble'
> +
> + Preamble and postamble for sitemap. Useful for inserting
> + #+OPTIONS: keywords, footers etc. See
> + `org-publish-sitemap-preamble' for details.
Note there are not much details in `org-publish-sitemap-preamble' either.
> +(defcustom org-publish-sitemap-preamble nil
> + "Sitemap preamble.
> +
> +Can be either a string, a list of strings, or a function that
> +takes a project-plist as an argument and return a string."
"returns"
Is allowing both strings and lists of strings useful enough?
> +Can be either a string, a list of strings, or a function that
> +takes a project-plist as an argument and return a string."
See above.
> + (sitemap-title (or (plist-get project-plist :sitemap-title)
> + (concat "Sitemap for project " (car project))))
> + (prepare-pre-or-postamble (lambda (pre-or-postamble)
I suggest to move lambda on the next line and use a less mouthful name
for the argument.
> + (cond ((not pre-or-postamble) nil)
> + ((stringp pre-or-postamble) pre-or-postamble)
> + ((listp pre-or-postamble)
> + (mapconcat 'identity preamble "\n"))
#'identity
> + ((functionp pre-or-postamble)
> + (funcall pre-or-postamble project-plist))
> + (t (error (concat "unknown `:sitemap-preamble' or "
> + "`:sitemap-postamble' format")))))))
I think `concat' is not necessary with the indentation rule suggested
above. Also: "Unknown".
> + ;; Call function to build sitemap based on files and the project-plist.
> + (let* ((style (or (plist-get project-plist :sitemap-style) 'tree))
> + (fun (intern (format "org-publish-org-sitemap-as-%s" style))))
Side note : I think this is a bit too smart. It prevents, e.g., from
grepping for a function name. Maybe
(if (eq style 'something) #'... #'....)
would be better.
> + (unless (functionp fun)
> + (error "Unknown function %s for :sitemap-style %s."
> + fun style))
I know this is not directly related to your patch, but it should
probably be `user-error'. Also, error messages are not expected to end
on a period.
> +@item @code{:sitemap-preamble}
> +@item @code{:sitemap-postamble}
> +@tab Preamble and postamble for sitemap. Useful for inserting
> + @code{#+OPTIONS}, footers etc. See @code{org-publish-sitemap-preamble}
> + for details.
I don't understand the "useful for inserting @code{#+OPTIONS}" part.
Maybe some examples could be useful. This part of the manual is rather
terse.
Regards,
--
Nicolas Goaziou
next prev parent reply other threads:[~2016-06-01 15:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-19 15:39 [ox-publish, patch] More flexible sitemaps Rasmus
2016-05-22 22:58 ` Nicolas Goaziou
2016-05-27 16:41 ` Rasmus
2016-06-01 15:34 ` Nicolas Goaziou [this message]
2016-07-05 11:08 ` Robert Klein
2016-07-06 11:17 ` Rasmus
2016-07-07 9:03 ` Rasmus
2016-07-20 7:56 ` Nicolas Goaziou
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=87wpm8j527.fsf@saiph.selenimh \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=rasmus@gmx.us \
/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).