emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Eric Schulte <schulte.eric@gmail.com>
Cc: Mark Edgington <edgimar@gmail.com>,
	emacs-orgmode <emacs-orgmode@gnu.org>,
	Nicolas Girard <girard.nicolas@gmail.com>
Subject: Re: proposal to have ignoreheading tags/properties
Date: Mon, 16 Jun 2014 10:08:45 +0200	[thread overview]
Message-ID: <87oaxtmg36.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87zjhdk63p.fsf@gmail.com> (Eric Schulte's message of "Sun, 15 Jun 2014 21:14:22 -0400")

Hello,

Eric Schulte <schulte.eric@gmail.com> writes:

> In my opinion the manual interleaving of "noexport" and "export" tags is
> overly cumbersome and is non-obvious.

It is as non-obvious as the task it achieves.

> The obscure nature of this solution is evidenced by multiple
> discussions and implementations of filter functions to handle
> situations which could be covered by this noexport/export pattern.

I don't know what you are referring to, since this "noexport/export"
pattern isn't implemented yet. Do you have an example of such
a discussion that dismissed said pattern?

> I think the attached patch should be applied to the core.  It includes
> the following.
>
> - a single new export function which removes the headlines and contents
>   (the "section" element) of headlines tagged "ignoreexport", then
>   retains and promotes all sub-headlines contained therein.
>
> - a single new export tag named "ignoreexport" (ignore seems to be the
>   crowd favorite, and adding "export" reduces possible conflicts with
>   existing personal tags)

Thank you for the patch.

Again, my concern is that it introduces another tag for a task that
could be possible without it. "export", "noexport", "ignoreexport": this
all gets confusing. Furthermore, this tag should, at the bare minimum,
be configurable so it also introduces another keyword on par with
SELECT_TAGS and EXCLUDE_TAGS.

Moreover, that task is highly specific; I'm not convinced we should have
a dedicated keyword for each of them. I'd rather have a simple solution
for selective export problems, even if, as a generic solution, it may
look clumsier.

> From the included tests, the effect of this patch is to convert a tree
> like the following,
>
> ,----
> | * H1
> | Text1
> | ** H2                                                          :ignoreexport:
> | Text2
> | *** H3
> | Text3
> | **** H4
> | Text4
> | *** H5
> | Text5
> | **** H6                                                        :ignoreexport:
> | Text6
> | ***** H7
> | Text7
> | ***** H8
> | Text8
> `----

Note that my suggestion is

  ,----
  | * H1
  | Text1
  | ** H2                                                          :noexport:
  | Text2
  | *** H3                                                           :export:
  | Text3
  | **** H4
  | Text4
  | *** H5                                                           :export:
  | Text5
  | **** H6                                                        :noexport:
  | Text6
  | ***** H7                                                         :export:
  | Text7
  | ***** H8                                                         :export:
  | Text8
  `----

In this case, it is more verbose. But the opposite would happen if you
needed to also ignore children within the "ignoreexport" headlines.
In order to get

  * H1
  ** H4

compare


  * H1
  ** H2   :ignoreexport:
  *** H3      :noexport:
  *** H4

with

  * H1
  ** H2       :noexport:
  *** H3
  *** H4        :export:

This time, the brevity of :ignoreexport: is not so clear. Also, it will
not prevent the need to nest tags at some points. IOW, it is only
efficient for one very specific use case. Outside of this case, the new
tag would just be an annoyance and you would fallback
to :noexport:/:export: nesting, if implemented. And if we cannot avoid
tags nesting, I'd take 2 "nesteable" tags over 3 any day.

Another point to consider is that :noexport:/:export has the advantage
to mark explicitly what will be included in the export process. As such,
I don't find less obvious than your proposal: in both cases, you have to
understand the rules applied behind the keywords anyway. And these rules
are well-known for "export" and "noexport" tags.

> I'm sympathetic to arguments about maintaining simplicity of the core,
> and even more so to arguments about maintaining structural validity of
> trees during export.  I believe that this revised patch fully maintains
> valid tree structures, and I'd suggest that the increase in complexity
> of a single keyword is justified by the demonstrated user demand for
> this functionality.

It mostly preserves tree structure, indeed. More comments about this
below.

Just to be clear: I'm not against implementing the functionality. We can
implement it without increasing complexity, so the demand is hardly
a justification for this added complexity.

Making :export: and :noexport: tags symmetrical is a good thing anyway:
why could we add :noexport: within :export: trees but not the other way?
And, as I demonstrated earlier, combination of both permits more
patterns than :ignoreexport:. Therefore, we should implement it in any
case. As a bonus, it provides a solution for the problem at hand.

If this solution happens to be inadequate, then we can discuss about
better solutions, this time with more options (a user-defined hook,
a macro, a command, even a new tag if we cannot escape it...).

Anyway, some comments follow about your code.

> +(defun org-export-ignore-headlines-retain-and-promoting-children (data info)
> +  "Remove headlines tagged \"ignoreexport\" retaining sub-headlines.
> +DATA is the parse tree.  INFO is a plist containing export
> +options.  Each headline tagged \"ignoreexport\" will be removed
> +removing its contents but retaining and promoting any children
> +headlines by a single level."
> +  (org-element-map data org-element-all-elements

Since you're only interested in headlines, org-element-all-elements -->
headline

> +    (lambda (object)
> +      (when (and (equal 'headline (org-element-type object))

This check is not necessary per the suggestion above.

> +                 (member "ignoreexport" (org-element-property :tags object)))
> +        (mapc (lambda (el)
> +                ;; recursively promote all nested headlines
> +                (org-element-map el 'headline
> +                  (lambda (el)
> +                    (when (equal 'headline (org-element-type el))

You don't need this `mapc' + `org-element-map' + `org-element-type'
construct. More on this below.

> +                      (org-element-put-property el
> +                        :level (1- (org-element-property :level el))))))

Actually, hard-coding `1-' will not always work, because it assumes that
children differ from their parent by exactly one level. This is not
always the case.

 * 1
 *** 1.0.1
 ***** 1.0.1.0.1
 ***** 1.0.1.0.2

Of course, this is a contrived example. Nevertheless, just to make it
robust, promoting children should put the lowest level children to the
level of their parent and preserve relative level differences between
other children.

> +                ;; insert back into parse tree
> +                (org-element-insert-before el object))
> +              ;; drop first three elements of headline
> +              ;; 1. headline tag
> +              ;; 2. properties
> +              ;; 3. section
> +              (cdddr object))

`cddr' is really `org-element-contents'. Also, some headlines do not
have a section, which is not mandatory. In this case `cdddr' would also
remove the first headline, which is wrong.

This can be solved by replacing `mapc' + `org-element-map' + `cdddr'
with:

  (org-element-map (org-element-contents object) 'headline
    (lambda (el) ...))


I appreciate your work on this, but let me stress it again: I'm
convinced that :ignoreexport: tag is only a partial solution to the
problem, and, as such, a dead-end. :noexport:/:export: combination is
more versatile, at the price of some verbosity in some cases, but
without the added complexity another tag would introduce.


Regards,

-- 
Nicolas Goaziou

  reply	other threads:[~2014-06-16  8:08 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-12 16:49 proposal to have ignoreheading tags/properties Mark Edgington
2014-06-12 17:32 ` Thorsten Jolitz
2014-06-12 17:41   ` Ken Mankoff
2014-06-12 18:11     ` Thorsten Jolitz
2014-06-12 18:16       ` Ken Mankoff
2014-06-13 14:32       ` Rasmus
2014-06-13 15:02         ` Thorsten Jolitz
2014-06-12 18:09   ` Mark Edgington
2014-06-12 18:12 ` Eric Schulte
2014-06-12 18:54   ` Aaron Ecay
2014-06-12 19:21     ` Nicolas Girard
2014-06-12 19:26       ` Ken Mankoff
2014-06-12 19:52         ` Nicolas Girard
2014-06-13  1:20         ` Samuel Wales
2014-06-12 19:34       ` Nicolas Girard
2014-06-12 20:13       ` Eric Schulte
2014-06-12 22:42         ` Nicolas Girard
2014-06-12 23:36           ` Eric Schulte
2014-06-13  0:35         ` Ken Mankoff
2014-06-13  0:46           ` Eric Schulte
2014-06-13  2:35             ` Ken Mankoff
2014-06-13 11:11               ` Eric Schulte
2014-06-13  3:28             ` Mark Edgington
2014-06-13 14:23         ` Rasmus
2014-06-14 12:43         ` Nicolas Goaziou
2014-06-14 16:48           ` Mark Edgington
2014-06-14 18:12             ` Aaron Ecay
2014-06-14 18:12             ` Nicolas Goaziou
2014-06-14 18:07           ` Aaron Ecay
2014-06-14 18:22             ` Nicolas Goaziou
2014-06-14 22:39               ` Aaron Ecay
2014-06-16  1:14           ` Eric Schulte
2014-06-16  8:08             ` Nicolas Goaziou [this message]
2014-06-16 12:19               ` Mark Edgington
2014-06-16 13:29               ` Eric Schulte
2014-06-22  2:03                 ` Aaron Ecay
2014-06-22 23:52                   ` Eric Schulte
2014-07-27 17:21                     ` Bastien
2014-07-28 18:15                       ` Mark Edgington
2014-07-28 18:27                       ` Rasmus
2014-07-28 19:21                         ` Mark Edgington
2014-07-28 19:43                       ` Nicolas Goaziou
2014-07-28 22:01                         ` Rasmus
2014-07-29 14:31                         ` Bastien
2014-08-02  5:16                           ` Mark Edgington
2014-08-06  4:09                             ` Aaron Ecay
2014-06-13  2:38 ` Eric Abrahamsen
2014-06-13  4:07   ` Mark Edgington
2014-06-13  4:44     ` Eric Abrahamsen

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=87oaxtmg36.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=edgimar@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=girard.nicolas@gmail.com \
    --cc=schulte.eric@gmail.com \
    /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).