bastibe / org-static-blog

A static site generator using org-mode
BSD 3-Clause "New" or "Revised" License
341 stars 74 forks source link

Preview markers #95

Closed jaor closed 3 years ago

jaor commented 3 years ago

This adds markers for preview start/end, as discussed in issue #68

jaor commented 3 years ago

i've just also added a custom option to print dates first, which fixes all my problems with #68.

jaor commented 3 years ago

On Wed, Mar 24 2021, Bastian Bechtold wrote:

In org-static-blog.el:

@@ -182,6 +182,16 @@ See also `org-static-blog-preview-ellipsis' and :type '(boolean) :safe t)

+(defcustom org-static-blog-preview-start nil

  • "Marker indicating the beginning of a post's preview."

This should probably include a note about the meaning of nil, much like in org-static-blog-rss-max-entries. I'm assuming if nil, the "legacy" behavior of the first paragraph is used?

Yes, you're right. Notes added.

In org-static-blog.el:

@@ -182,6 +182,16 @@ See also `org-static-blog-preview-ellipsis' and :type '(boolean)

Should perhaps add a reference to the variables org-static-blog-preview-start and org-static-blog-preview-end, to clarify the behavior with and without them.

i've added a note. given that now those variables have a full explanation of their behaviour, i refer to their docstring as that explanation. please see if that feels clear enough.

In org-static-blog.el:

@@ -182,6 +182,16 @@ See also `org-static-blog-preview-ellipsis' and :type '(boolean) :safe t)

+(defcustom org-static-blog-preview-start nil

  • "Marker indicating the beginning of a post's preview."
  • :type '(choice (const :tag "First paragraph" nil) (string))
  • :safe t)
  • +(defcustom org-static-blog-preview-end nil

  • "Marker indicating the end of a post's preview."

This should probably include a note about the meaning of nil, much like in org-static-blog-rss-max-entries. I'm assuming if nil, the "legacy" behavior of the first paragraph is used?

done

What happens if only one of org-static-blog-preview-start and org-static-blog-preview-end is set?

the other one uses the 'default' paragraph marker... i've tried to clarify this a bit in the docstrings.

In org-static-blog.el:

@@ -445,40 +455,51 @@ e.g. (('foo' 'file1.org' 'file2.org') ('bar' 'file2.org'))" (push (cons tag (list post-filename)) tag-tree))))) tag-tree))

+(defun org-static-blog--preview-region ()

  • "Find the start and end of the preview in the current buffer."

It looks like this function could be made simpler, by first assigning a start marker and an end marker, which are (or org-static-blog-preview-start "

") and (or org-static-blog-preview-end "

"), respectively.

After that, you could forego the big decision tree and do a static search for the area between them.

But I might be wrong on that, perhaps I am missing some nuance. That's just the impression I got from reading your implementation.

yes, there are several ways of making this body more concise, but, at least to me, those ways obscure the logic of what's going on, and i produced unintended behaviours in a couple of simplifications... so i thought it was better to be redundant for the sake of clarity (even though the clarity i attained seems up to debate :)) but if you feel strongly about it, i can give it another try, let me know.

In org-static-blog.el:

@@ -445,40 +455,51 @@ e.g. (('foo' 'file1.org' 'file2.org') ('bar' 'file2.org'))" (push (cons tag (list post-filename)) tag-tree))))) tag-tree))

+(defun org-static-blog--preview-region ()

  • "Find the start and end of the preview in the current buffer."
  • (goto-char (point-min))
  • (if org-static-blog-preview-end
  • (when (or (search-forward (or org-static-blog-preview-start "

    ") nil t)

  • (search-forward "

    " nil t))

  • (let ((start (match-beginning 0)))
  • (or (search-forward org-static-blog-preview-end nil t)
  • (search-forward "" nil t))

  • (cons start (point))))
  • (when (search-forward (or org-static-blog-preview-start "

    ") nil t)

  • (let ((start (match-beginning 0)))
  • (search-forward "")

  • (cons start (point))))))

It might simplify things to return (buffer-substring-no-properties start (point)) here directly, instead of the two positions.

good idea. done.

If org-static-blog--preview-region returned a string instead of two positions, this let* could be reverted to a straight let, and the code would simplify a bit.

yes, it's better now. i've pushed the changes above in 515fcba, together with a README update that mentions these new variables and a couple more i introduced in previous PR. let me know how it looks.

bastibe commented 3 years ago

Brilliant! I think you addressed all pertinent points.

there are several ways of making this body more concise, but, at least to me, those ways obscure the logic of what's going on, and i produced unintended behaviours in a couple of simplifications...

That's all right then. It is often difficult to appreciate the complexity of innocuous-looking code.

The PR looks good to me. I'll happily merge it if that's ok with you!

jaor commented 3 years ago

On Sun, Mar 28 2021, Bastian Bechtold wrote:

The PR looks good to me. I'll happily merge it if that's ok with you!

Absolutely ok :) Thanks!

bastibe commented 3 years ago

Thank you very much for your contribution, then!

(Feel free to ping me on this thread if this PR is not included in a release within a few weeks, or earlier if need it.)

jaor commented 3 years ago

Thanks Bastian, it's always a pleasure to be able to contribute. I'd like to take the chance to ask about a further feature i was thinking about. I think it's been asked before whether it'd be possible to have "pages" other than blog posts in the site, and the natural answer is negative.

But it occurred to me that we could just use the same "no-rss tag" trick we already have in place for those case. Right now, any post with that tag won't appear in the RSS feeds. We could have a "no index" tag that would indicate that that page shouldn't appear in the index pages, and that way one has complete control over where posts appear. The implementation, being a filter on tags similar to the one we already have in place for no-rss, should be really simple.

Is that something you'd consider acceptable?

bastibe commented 3 years ago

We already have org-static-blog-rss-excluded-tag, as well as the drafts directory. Personally, I like to use the drafts directory for this purpose, as drafts appear neither in the RSS feed, nor in the archive or index pages.

jaor commented 3 years ago

On Mon, Mar 29 2021, Bastian Bechtold wrote:

We already have org-static-blog-rss-excluded-tag, as well as the drafts directory. Personally, I like to use the drafts directory for this purpose, as drafts appear neither in the RSS feed, nor in the archive or index pages.

yes, but there's a missing combination: pages appearing in the RSS feed but not in the indexes. drafts would be the equivalent of specifying both org-static-blog-rss-excluded-tag and then new org-static-blog-index-excluded-tag. admittedly, it's not a frequent use case, but there's an argument for symmetry, i guess. also, that'd allow one to have "drafts" (= posts with those two tags) in any directories.

bastibe commented 3 years ago

I get what you are saying, now! Sorry for reading your message too quickly, and not catching on to what you were saying.

Yes, a new org-static-blog-index-excluded-tag would indeed be useful, and I would definitely enjoy a pull request that implemented it.

On a similar note, it might be useful to have tags for disabling comments and dates. Although with such a proliferation of tags, I wonder of #+filetags: is indeed an appropriate mechanism for implementing them. It might be preferable to specify them as

#+PROPERTY: no-index
#+PROPERTY: no-rss
#+PROPERTY: no-comments
#+PROPERTY: no-date

or perhaps

#+PROPERTY: exclude-from index rss archive
#+PROPERTY: exclude-elements comments date

(I am just thinking out loud, here, based on the possibilites mentioned in the documentation)

What are your thoughts on this?

jaor commented 3 years ago

hmm, i think filetags is more flexible and simpler than a collection of properties.

imagine that tomorrow i decide that i want all my posts tagged "foo" to disappear from the index: with tags, that's just customizing a variable, with properties i'd have to go one by one. we could also easily extend variables such as org-blog-index-excluded-tag to accept more than one tag, making those possibilities even more flexible (for instance, imagine i want to exclude the "2020" and "2019" tags starting today).

also, headers for posts with (multiple) exclusions are shorter and (imo) tidier with a couple more tags than with new #+property lines.

along those lines (and similarly to your second exclude-from properties) a thing that might be worth thinking about is having a single excluding variable, instead of four... something like org-static-blog-exclusion-tags which is an alist:

    ((index "noindex" "program")
     (rss "norss")
     (comments "closed" "old")
     (date "undated"))

with that syntax it's easy to have multiple tags, and it's easy to exted things if we think of other exclusions in the future.

your distinction between exclude-from and exclude-elements makes logical sense to me, so one could also have two alists, instead of cramming all together in one, but there's the tradeoff that a single variable is simpler, so i am not really sure about it (if we went for properties, i think i'd like two separate ones better, though).

(one could be even more flexible, and have a customizable /function/ that takes the list of tags and properties of a post, and returns the elements to be published... in fact, when implementing this such a function will possibly already be there (it'll take the alist above, or the names of the properties and do a lookup); if so, it won't probably do any harm to make it customizable for advanced users).

also thinking aloud here... what do you think?

jaor commented 3 years ago

wanted to add: a drawback of the tags idea is that we're mixing things that are logically separate (element exclusions, categories) in a single place (filetags), so even though it's practical, one could argue that it's also a dirty solution :) but i'm still liking that ability to being able to change exclusions of groups of posts by customizing a single variable...

bastibe commented 3 years ago

I see your point that it is useful to be able to re-use existing tags for exclusion rules. As I see it, it's a bit of a trade-off between being pragmatic (reuse existing functionality) and being pedantic (separate functionality by purpose). While I lean towards the latter, I am not a user of these sorts of features, and would defer judgement to those who actually use it. I'm fine with the pragmatic approach, is what I am saying.

I like your idea of a combined variable, which could take a more general form, such as:

((exclude-from-index "noindex" "program")
 (exclude-from-rss "norss")
 (no-comments "closed" "old")
 (no-date "undated" "standalone")
 (no-tags "untagged" "standalone")
 (invisible-tags "noindex" "program" "norss" "closed" "old" "undated"))

I added no-tags for hiding the tag list, and invisible-tags, which are tags that should not show in the tag list. The latter I think is necessary if we're introducing many such non-category tags.

Or perhaps I am overthinking things here. Perhaps the exclude-from things are well-suited for tags, while the no- flags are better relegated to a #+property, as they would generally be applied per-file instead of for a group of files (I'd imagine).

Would you like to put together a new PR or issue with a draft for this feature? Any further comments would also be welcome of course.

jaor commented 3 years ago

On Wed, Mar 31 2021, Bastian Bechtold wrote:

I see your point that it is useful to be able to re-use existing tags for exclusion rules. As I see it, it's a bit of a trade-off between being pragmatic (reuse existing functionality) and being pedantic (separate functionality by purpose). While I lean towards the latter, I am not a user of these sorts of features, and would defer judgement to those who actually use it. I'm fine with the pragmatic approach, is what I am saying.

I like your idea of a combined variable, which could take a more general form, such as:

((exclude-from-index "noindex" "program") (exclude-from-rss "norss") (no-comments "closed" "old") (no-date "undated" "standalone") (no-tags "untagged" "standalone") (invisible-tags "noindex" "program" "norss" "closed" "old" "undated"))

I added no-tags for hiding the tag list, and invisible-tags, which are tags that should not show in the tag list. The latter I think is necessary if we're introducing many such non-category tags.

i like the general structure you propose above. i had not thought about invisible-tags: i was thinking of making all these special tags invisible. maybe that could be the default if no invisible-tags entry is provided? in that generic form, how would this variable be named? org-static-blog-tag-categories?

(if we finally introduce this one, i guess we should at least deprecate org-static-blog-rss-excluded-tag... or remove it altogether, i might well still be its only user :))

bastibe commented 3 years ago

I think org-static-blog-tag-categories could be confused with post Categories. Perhaps org-static-blog-tag-classes? Or in keeping with hiding them all by default, org-static-blog-special-tags?

You are probably correct that they should be hidden by default, although we might want to make this configurable.

And I agree with you that org-static-blog-rss-excluded-tag should be deprecated, although it should continue to work if we insert it into the above variable as ... (exclude-from-rss org-static-blog-rss-excluded-tag) ... by default. (This will necessitate that these entries might contain multiple nils, but that's probably a prudent practice anyway)

jaor commented 3 years ago

On Sat, Apr 03 2021, Bastian Bechtold wrote:

I think org-static-blog-tag-categories could be confused with post Categories. Perhaps org-static-blog-tag-classes? Or in keeping with hiding them all by default, org-static-blog-special-tags?

yes, i like static-blog-special-tags.

You are probably correct that they should be hidden by default, although we might want to make this configurable.

yes. but if we include the hidden class in org-static-blog-special-tags we already have that configurability, right? i mean, you don't mean having a special, separate flag for that?

And I agree with you that org-static-blog-rss-excluded-tag should be deprecated, although it should continue to work if we insert it into the above variable as ... (exclude-from-rss org-static-blog-rss-excluded-tag) ... by default. (This will necessitate that these entries might contain multiple nils, but that's probably a prudent practice anyway)

sounds good.

bastibe commented 3 years ago

but if we include the hidden class in org-static-blog-special-tags we already have that configurability, right? i mean, you don't mean having a special, separate flag for that?

Frankly, I'll leave that up to you. I can see value in having certain tags both as regular tags and, say, exclude comment boxes ("politics"). But since I am not a user of features such as these, either way is fine for me.

jaor commented 3 years ago

i was giving this a try and i'm finding the alist idea overkill. we only need a tag for no-rss and another for no-index, which means introducing just another variable, and for the undated, untagged and closed cases, which as you mention are more likely to apply to individual posts, the #+PROPERTY: no-date, etc. properties you propose seem simpler too... i'll send a prototype in a separate PR so that we can see how it looks, once i find some time.

bastibe commented 3 years ago

Great, I'll look forward to the PR!