Request for feedback: ox-linuxmag

Request for feedback: ox-linuxmag

[Damien Cassou]

Hi,

I'm done writing ox-linuxmag, an exporter for the French GNU/Linux
Magazine: https://github.com/DamienCassou/ox-linuxmag.

I would be happy to receive feedback.

Thank you for your help during the implementation of this exporter.

Best,

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

Re: Request for feedback: ox-linuxmag

[Ihor Radchenko]

Damien Cassou <damien@cassou.me> writes:

> I'm done writing ox-linuxmag, an exporter for the French GNU/Linux
> Magazine: https://github.com/DamienCassou/ox-linuxmag.

Why not using org-export-define-derived-backend?
Also, linuxmag might be confused with
https://www.linux-magazine.com/tags/view/Operating+Systems/GNU

> (defconst ox-linuxmag--resources-dir
>   (expand-file-name "resources" (file-name-directory (or load-file-name (buffer-file-name))))
>   "Directory containing the style and template XML files.")

This will spoil the installation dir.

> ox-linuxmag--template

Your template function has side effects. Side effects are usually
implemented inside the export function. like ox-linuxmag--export-to-odt

> ox-linuxmag--export-to-odt

Why is it marked as private function? By convention, Org exporters
provide public function to export programatically.

And it is (interactive), which is even more strange given that it is
marked private (--).


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Request for feedback: ox-linuxmag

[Damien Cassou]

Ihor Radchenko <yantar92@posteo.net> writes:
> Why not using org-export-define-derived-backend?

I didn't know this function exist. I changed the code to use it.

> Also, linuxmag might be confused with…

What do you think about ox-linuxmag-fr as package name and linuxmag-fr
as exporter name?

>> (defconst ox-linuxmag--resources-dir
>>   (expand-file-name "resources" (file-name-directory (or load-file-name (buffer-file-name))))
>>   "Directory containing the style and template XML files.")
>
> This will spoil the installation dir.

What do you mean by that? Do you have a different way to do it? The odt
exporter contains a similar variable and external files (see
`org-odt-lib-dir`).

> Your template function has side effects. Side effects are usually
> implemented inside the export function. like ox-linuxmag--export-to-odt

I guess you are referring to `ox-linuxmag--write-meta-file` and
`ox-linuxmag--write-styles-file`. I would be happy to move them to the
export function but they depend on `info` and I don't know how to get
that from there. Also, I copy/pasted the structure of ox-odt which does
something similar in `org-odt-template`.

Any recommendation?


>> ox-linuxmag--export-to-odt
>
> Why is it marked as private function? By convention, Org exporters
> provide public function to export programatically.

You are perfectly right. This is now fixed.

Thank you very much for helping me improve the code! I really appreciate
the time and effort.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

Re: Request for feedback: ox-linuxmag

[Ihor Radchenko]

Damien Cassou <damien@cassou.me> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>> Why not using org-export-define-derived-backend?
>
> I didn't know this function exist. I changed the code to use it.

See A.4 Adding Export Back-ends section in Org manual ;)

>> Also, linuxmag might be confused with…
>
> What do you think about ox-linuxmag-fr as package name and linuxmag-fr
> as exporter name?

Sounds more accurate.

>>> (defconst ox-linuxmag--resources-dir
>>>   (expand-file-name "resources" (file-name-directory (or load-file-name (buffer-file-name))))
>>>   "Directory containing the style and template XML files.")
>>
>> This will spoil the installation dir.
>
> What do you mean by that? Do you have a different way to do it? The odt
> exporter contains a similar variable and external files (see
> `org-odt-lib-dir`).

You are right. I somehow thought that you are writing to that directory.

>> Your template function has side effects. Side effects are usually
>> implemented inside the export function. like ox-linuxmag--export-to-odt
>
> I guess you are referring to `ox-linuxmag--write-meta-file` and
> `ox-linuxmag--write-styles-file`. I would be happy to move them to the
> export function but they depend on `info` and I don't know how to get
> that from there. Also, I copy/pasted the structure of ox-odt which does
> something similar in `org-odt-template`.
>
> Any recommendation?

I think :filter-final-output in :filters-alist will be more suitable.
It is called at the very end and may set some let-bound value to be used
in the ox-linuxmag-export-to-odt.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Request for feedback: ox-linuxmag

[Damien Cassou]

Hi Ihor,

Ihor Radchenko <yantar92@posteo.net> writes:
> I think :filter-final-output in :filters-alist will be more suitable.
> It is called at the very end and may set some let-bound value to be
> used in the ox-linuxmag-export-to-odt.

I'm not really sure what you mean with the last sentence. In my last
commit, I made use of :filter-final-output:

https://github.com/DamienCassou/ox-linuxmag-fr/commit/195f154d5f76f50a94e4bdc95f27c8400e77992d

(please note the new URL of the project reflecting its new name).

As you can see, I didn't change ox-linuxmag-fr-export-to-odt. Do you
suggest some changes there as well?

I took care of all your other feedback. Thank you so much for taking the
time to help me.

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

Re: Request for feedback: ox-linuxmag

[Ihor Radchenko]

Damien Cassou <damien@cassou.me> writes:

> Hi Ihor,
>
> Ihor Radchenko <yantar92@posteo.net> writes:
>> I think :filter-final-output in :filters-alist will be more suitable.
>> It is called at the very end and may set some let-bound value to be
>> used in the ox-linuxmag-export-to-odt.
>
> I'm not really sure what you mean with the last sentence. In my last
> commit, I made use of :filter-final-output:
>
> https://github.com/DamienCassou/ox-linuxmag-fr/commit/195f154d5f76f50a94e4bdc95f27c8400e77992d
>
> (please note the new URL of the project reflecting its new name).
>
> As you can see, I didn't change ox-linuxmag-fr-export-to-odt. Do you
> suggest some changes there as well?

I think you misunderstood what I meant.

(Note that this particular suggestion is more about code style and
ensuring side-effect-free export if something fails).

The idea is that before org-export processed the Org buffers and
produces the final form of the converted output, nothing should touch
the filesystem. Otherwise, if the export process fails for some reason
(like user filter erring), some garbage may be produced and left on the
disk.

So, my suggestion is to set some transient data in your custom output
filter. Then, inside ox-linuxmag-fr-export-to-odt, you can reuse the
data to write all the necessary files on disk knowing that nothing
failed before.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>