search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 548 forks source link

[feature] Add =image to perl pod #18169

Open dk opened 4 years ago

dk commented 4 years ago

I propose an extension that, in spirit of "easy things easy", makes embedding of images in pod, well, easy. Currently it is possible with a lots of ugly hacks, but the worst is especially knowing the formatted beforehand - f.ex. to show such a pod in github or metacpan requires magic like

=for html <p><img src="...png">

where you need to know that it is html, for example.

(see https://metacpan.org/pod/distribution/Prima/pod/Prima/Drawable.pod#Font-ABC-metrics as an example. It works but it is ugly).

In my experience an image tag should be able to do the following: 1) display a locally installed image 2) display a remote image (as in "img src") 3) display a text fallback (as in "img alt" or similar) 4) display nothing

An =image tag, in full form, would be thus used like this, for example:

=image begin

=image local "lib/graphic.png" (get searched in @INC for example)

=image remote "http://github.com/.../graphic.png"

^ y
|       *
| *
|------------> x

graphic for man pages

=image end

and also a short syntax for 90% of cases just as

=image "http://github.com/.../graphic.png"

This will break backward compatibility, but I'm decidedly not proposing a weaker but compatible option, such as f.ex.

=begin image 

=for image 

=end image

because I think that images should no longer be hacks but rather 1st class citizens.

Let's discuss this

Grinnz commented 4 years ago

I appreciate the design proposed, and =for html is awkward to use for this (though in practice HTML is the only medium that actually renders images), but I would much rather convince Pod::Simple::XHTML to recognize =for image which does not need to change the spec or break compatibility, and only would affect formatters that care about it.

xsawyerx commented 4 years ago

General comments:

I have a lot of questions that come up:

My biggest concern here is that it will promote a single, specific multimedia element as first-class in the POD specification. There are both additional types of multimedia elements, as well as elements that can replace this one. So why this one? What about them?

My second biggest concern is that we might be growing here into an attempt to recreate arbitrarily complicated HTML using POD and I'm not sure we'll be able to do it. (It might not be exactly "arbitrarily complicated," but it sure can be. The example in the ticket includes <p> tags, so we're definitely trying to replicate paragraphs along with the <img> tag.)

dk commented 4 years ago

@Grinnz I agree that breaking back compatibility is a thing better to avoid, but I also don't see a problem introducing it as it happened already with =head3 and =head4 back then. If old formatters emit a warning f.ex. "tag =image unknown" but will continue working that is even better I think, that way they send a signal to the user that update is needed

dk commented 4 years ago

@xsawyerx I see a need for local images, f ex when converting to latex or pdf, or using an offline documentation. I agree that local and remote subtags are somewhat inelegant, however I seriously doubt that one can (and for that matter, should) enforce a html-like schema where both local and remote images can be defined as a single unambiguous URI. F ex metacpan or github, when rendering a pod file as html, won't extract referenced images that are found correspondingly either in archive or in a repository, that might have different paths compared to where an image is to be installed locally.

What I meant by INC is to define a local storage for a module, which is in my eyes falls quite naturally in $SITEPERL/lib/$MODULE . Many modules already install pod files there - would you consider that a malpractice too? I don't mind though extending =image with location rules other than INC, as long as this is standartized; I wish though this could be done easily because when one runs "perldoc foo", foo.pod is searched exactly in INC.

However I very much agree that =image and potential friends like =video could indeed begin to grow into an unmanageable set of features. That's why I'd rather keep =image spartan, confining it to the 4 options I outlined in the ticket: local, remote, text fallback, and no text fallback. Ah yes, a figcaption too; figcaption is nice.

haarg commented 4 years ago

Previous discussion: metacpan/metacpan-web#1965

xsawyerx commented 4 years ago

Are you suggesting that we decouple the concept of an image from HTML's img property?

If our image property would correlate to the img property in HTML, we would first need to support all the fields it has in a consistent manner (local vs. remote is managed using the implicit URI spec in browsers), we would need to figure out how to support the figure and figcaption options - oh, and maybe srcset too.

If we're not coupling the suggested image property from HTML's img property, we would still need a rich-enough specification to allow translating it into HTML. I think it makes sense for us to do this, but it might be a bigger undertaking to determine the superset of options.

It's worthwhile researching how this is handled in other formats. We could try to steal from TeX, but here lie dragons. TeX is far more capable than Pod and we might be stea^Winspired by TeX but still be unable to properly import the full capability it has (coughPerl6cough). In Markdown, there is a syntax for images but I fear it suffers from the same problems of elevating certain HTML elements to first-class.

Perhaps we could just follow Markdown in minimal support for images and the desire for more compatibility with Markdown to introduce it.

dk commented 4 years ago

I guess it was implicit, but yes, I do suggest to not copycat html's img tag and its properties. Moreover, I don't suggest to dive into devising a rich-enough specification, as this is going to open for 1) endless discussions and 2) will be too much tied up to html. For example, text support in html: it has all these great possibilities to set fonts, use text sub/superpositions, letter spacing, kerning and what not, and yet POD is just fine without these text features. So yes, I think that Markdown's minimal support for images is the way to go, Markdown did it right.

I'm not really proficient in TeX/LaTeX, but I know how to deal with images in PDFs. That format has a rather unique set of image capabilities not necessarily available in html - it can apply arbitrary matrices, stencil masks, alpha masks, porter-duff operators (special alpha blendings), create filling patterns from images, and more. I think supporting these would be a serious overkill, and that is the same point of view I take when arguing against supporting all of html image functionality.

tonycoz commented 4 years ago

@xsawyerx I see a need for local images, f ex when converting to latex or pdf, or using an offline documentation. I agree that local and remote subtags are somewhat inelegant, however I seriously doubt that one can (and for that matter, should) enforce a html-like schema where both local and remote images can be defined as a single unambiguous URI. F ex metacpan or github, when rendering a pod file as html, won't extract referenced images that are found correspondingly either in archive or in a repository, that might have different paths compared to where an image is to be installed locally.

Resolving relative references to images is something the parser will likely need to do (possibly as an option indicating a base URL for pod2html).

I think a simple relative URL rather then distinguishing local and remote URLs would work.

I expect most formatters would disable remote URLs by default to avoid remote images becoming an attack vector by default if some image library ends up with an attackable security issue. I'd be inclined to not support remote URLs at all, since some (most?) processors will need to fetch the URL to render the image, or even to just get the size for HTML.

While I'd love images in POD, I'm not sure they belong in the format.

dk commented 4 years ago

@tonycoz As my concern with remotes is only for metacpan/github/etc online renderers, I do agree that remotes, as a general concept, should be out of the picture. But let's look at github f ex: if I want to include an image from the same repository, I need this URL:

https://raw.githubusercontent.com/$USER/$MODULE/$BRANCH/pod/pic.png

whilte a pod itself, when rendered, is served from

https://github.com/$USER/$MODULE/tree/$BRANCH/pod/foo.pod

On one hand I'd love to say just =image pod/pic.png, on the other how would one persuade github to implement this?

dk commented 4 years ago

update:

1) just tested it on github, shows images just fine.

2) metacpan can display markdown files but can't show images

So I guess remote and local could be indeed merged! This is great. Which means that we could also borrow from the markdown syntax and make images part of a text paragraph, not a paragraph on its own - with syntax like P<title|uri.png>, for example. This would also cut attempts to extend on the image functionality (width, alpha, whatever), which I think is a good thing.

yuki-kimoto commented 4 years ago

because I think that images should no longer be hacks but rather 1st class citizens.

I think so. I also want pod syntax which can express image.

dk commented 4 years ago

Another point, the P<> syntax doesn't need to contain special text fallback. Because, contrary to the multitude of the graphical formatters, text can be addressed simply as =for text. That means one can write a pod such as


P<foo.png>

=for text this is graphic

if so desired.

I'm thinking about working on a patch however I don't want to find myself in a situation where it would be rejected or premature because the very topic is undecided yet. @xsawyerx what kind of consensus should be reached there for the green light, if any?

karenetheridge commented 4 years ago

Proposed changes to pod syntax should be run through the pod-people list before merging.

https://lists.perl.org/list/pod-people.html

tonycoz commented 4 years ago 

P<foo.png>

=for text this is graphic

The alt text could be expressed in the same way L<> does link text:

P<alt text|url>
dk commented 3 years ago

Hello again,

After running the proposal through pod people, the idea I'm thankful for as it highlighted the flaws I wasn't aware of, and so here's the next version of the proposal. The consensus became a yaml embedded into 'image' formatter, like this:

=begin image

src: /path/file.png
title: "Figure 1"
text: 
  multiline
  entry
resolution: 120

=end image

or, for 90% of cases, simply

=for image src:file.png
   title:"Figure 1"

Apparently the yaml is also TMTWODI about multiline entries ( see https://yaml-multiline.info/ ), and what's more interesting, cpan's YAML doesn't understand neither of these, but that's not really relevant because YAML.pm is not in the core. The proposal is agnostic about multiline style.

Pod people raised a concern about how to parse these YAML sections in core perl modules. It seems the most sensible solution would be to include a minimal Pod::Simple::YAML parser, just enough to parse 1-level deep structures found in the image section.

The "src:" tag is to be used for both local and remote path, referring to the installation and/or distribution root. It will be the module author's job to make sure that, the image referred is installed in the same relative path as it is found in the distribution.

The "text:" fallback will be produced by formatter that understands =image, but where graphic output is unavailable. The drawback is that old and non-conforming formatters will not display the fallback text

The "resolution:" tag is to be used when the image size is important to match the text size. Formatters may use the value, in DPI, to scale the output image accordingly.

The "title:" tag is to be displayed as a generic description of the image. For example XHTML may use <figcaption> for this. When graphic is unavailable, the tag content may not be displayed if the "text:" tag exists.

The format is open for more tags, however I think these should be conservative - potential addition of "width: 90%" and "rotation: 45" and what not will complicate the things greatly; the rationale is to keep syntax rather terse, in the same vein as markdown's image syntax.

In the output, an image will not be part of text, ie a floating image with f ex inline formula. An image will always end the current paragraph and will start a new one. Two =image blocks will be rendered as two separated paragraphs.

It is proposed to adjust the core Pod::Simple so that it knows about the =image block, and will parse it so that the formatters don't need doing so. Pod::Simple creates an element tree and passes it to the formatters; these, in turn, should be adjusted to produce the corresponding output.

Now, I hope I didn't forget anything. Kindly come with comments!

dk commented 3 years ago

@khwilliamson Karl, would that be okay to send a pull request for this feature in Pod::Simple?

haarg commented 3 years ago

Adding YAML to Pod seems like over-complicating things.

I had previously proposed a way to specify alternates where the Pod renderer would pick the first supported of several possible items it could render. The specific syntax I thought of probably isn't perfect. But that seems like it might be a better approach than creating a new way to embed text blobs in Pod.

dk commented 3 years ago

@haarg indeed, but that's apparently too limiting. There's no way to specify a title/footnote/figcaption which is really nice to have. Also, as pointed y pod-people, =for text is too limiting as well, it won't work for *roff f.ex.

khwilliamson commented 3 years ago

I do suggest sending a PR to Pod::Simple, and if people have comments on the specifics, they can do it there

dk commented 3 years ago

@khwilliamson Thank you, I (optimistically) understood your original answer as positive enough :)

Here's the work in progress that I believe already resolved all potential problems, only tests, docs, and polishing left: https://github.com/dk/pod-simple/commits/master/lib/Pod/Simple . The individual commits are rather untidy, as it's more for myself, and when everything is ready I'm planning to kill this repository, fork it anew, and submit a pull request as one huge commit -- unless you would prefer the commits as they are now.

New things introduced:

I also tested this on perl's Pod::Man that is not in the Pod::Simple distro, but it seems to be working.

You're very welcome to take a preliminary look.

dk commented 3 years ago

Hello everybody, as promised, there's a pull request sitting at https://github.com/perl-pod/pod-simple/pull/128/ . The fork is at https://github.com/dk/pod-simple, you're welcome to review and play with it.

The patch also opens the possibility to create html with images, but not by default. If you want to try pod2html you'll need this first:

--- lib/Pod/Html.pm     2020-10-24 13:09:44.661075012 +0200
+++ /usr/share/perl/5.30.0/Pod/Html.pm  2020-03-06 22:15:57.000000000 +0100
@@ -369,7 +369,6 @@
     my $parser = Pod::Simple::SimpleTree->new;
     $parser->codes_in_verbatim(0);
     $parser->accept_targets(qw(html HTML));
+    $parser->accept_image;
     $parser->no_errata_section(!$Poderrors); # note the inverse
dk commented 3 years ago

Hello everyone, the patch seems to be stalled for unclear reasons, and my invitation for discussion why doesn't seem to be working. I'm sorry I'm not really good with soft skills so I don't understand why is that. Would someone kindly look at the status and see whether it is possible to promote the patch, or restart the discussion, or something else?

xenu commented 3 years ago

FWIW, that's how feature requests in this project usually end up. We don't have a real feature proposal process, so in practice only committers get to propose and implement features.

xenu commented 3 years ago

Regardless of what I said above, while people generally seem to like the idea of =image, there's strong opposition to the proposed YAML-based syntax (especially in the Pod::Simple PR).

khwilliamson commented 3 years ago

And that is why I have yet to merge this. I haven't had time to educate myself as to the issues involved.

Leont commented 3 years ago

while people generally seem to like the idea of =image, there's strong opposition to the proposed YAML-based syntax

I think that is a fair assessment.

dk commented 3 years ago

Thanks everyone. It is though rather disheartening that the said opposition never uttered a word while the syntax was discussed and agreed on, and only did that after I put lots of efforts in the implementation and polishing and testing. I don't think I have energy for another round of discussion about what syntax would please the crown. Sorry.