Re: [NonGNU ELPA] New package org-transclusion-http

[Joseph Turner]

Philip Kaludercic <philipk@posteo.net> writes:

> Joseph Turner <joseph@ushin.org> writes:
>
>> Thank you for the review!
>>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>> There is really not much I can comment on, since it is mainly API I am
>>> not familiar with.  One note is that the package description doesn't
>>> make much sense to me, since I don't know what "transcluding" means.
>>> Also, not everyone knows that "plz" is a HTTP library (the name remains
>>> elusive), so a brief comment reminding the reader of that would be
>>> appropriate.
>>
>> I've updated the package description.
>
> This is the commit you are referring to?
>
> diff --git a/org-transclusion-http.el b/org-transclusion-http.el
> index 5f240ad31c..293e705615 100644
> --- a/org-transclusion-http.el
> +++ b/org-transclusion-http.el
> @@ -25,8 +25,8 @@
>  
>  ;;; Commentary:
>  
> -;; This file contains functionality related to transcluding content over HTTP
> -;; using plz.el.  Features include:
> +;; This file extends the `org-transclusion' package to allow for transcluding
> +;; content over HTTP.  Features include:
>  ;;
>  ;; - Transclude plain text
>  ;;   + Transclude only Org headings matching search options
>
> I don't know if this is just me, but I am not familiar with what
> "transcluding" is supposed to mean.  I get that it has something to do
> with networking, and if I had to guess you are fetching remote files and
> including them while exporting, but a package description shouldn't
> leave the reader with the impression that they have to guess what is
> going on.

Transclusion is the inclusion of part or all of an electronic document
into one or more other documents by reference via hypertext:
https://en.wikipedia.org/wiki/Transclusion

Since this package is just a minor extension of the org-transclusion
package, I think it's fine to direct users to learn more by reading the
package description for org-transclusion.

If there's a specific specific phrase you'd like to add, I'll be happy
to apply a patch.

>>> Another note is that if you already have a .dir-locals.el file, you
>>> could add a few more variables that might vary from system to system.
>>> In Compat, the .dir-locals.el lists:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> ((nil
>>>   (fill-column . 72)
>>>   (sentence-end-double-space . t)
>>>   (show-trailing-whitespace . t)
>>>   (indent-tabs-mode . nil)))
>>> --8<---------------cut here---------------end--------------->8---
>>
>> I've added all these variables except show-trailing-whitespace, as I
>> don't personally use it.  ws-butler takes care of whitespace without
>> adding visuals.
>
> No problem, I like `show-trailing-whitespace' because I know that all
> contributors will see that there is a whitespacing issue, but I admit
> that my recommendations might be opinionated.

Sounds good!

Thanks!

Joseph