Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image srcset support? #205

Open
tobiastom opened this issue May 26, 2015 · 13 comments
Open

Image srcset support? #205

tobiastom opened this issue May 26, 2015 · 13 comments

Comments

@tobiastom
Copy link

Hello,

are there an plans, ideas or best practices on how to use srcset within images?

I can see that it would be pretty hard to add this syntax to the markdown format, but maybe there could me some API extension that works like url_filter_func. Once an image is found, the API could define some (additional) attributes for the <img> tag that is generated.

Any ideas on this?

@michelf
Copy link
Owner

michelf commented May 26, 2015

That's not a bad idea. Though I wonder how such a filter function could look like, mostly what kind of return value would that be?

@tobiastom
Copy link
Author

I think the easiest solution would be to have a tag_handler which takes two parameters, both by reference I would propose:

handleTag( &$tagName, array &$attributes )

The $tagName is the name of the tag that would be used, e.g. img. $attributes would be a key value pair of attributes and values that would be added. Both parameters can be changed (that's why the by reference).

My tag handler could check for the tag name (if it is an image) and add the srcset attribute to the parameters, where nessesary.

This would be extreme extensible for other tags and attributes.

@michelf
Copy link
Owner

michelf commented May 26, 2015

A function like this one is going to have problems manipulating things that do not have a one-to-one mapping to HTML (things such as code blocks( <pre><code>), footnotes, tables). If genericness is what you want, we'd need to have something that takes parsed Markdown constructs and emits HTML, not something that takes HTML tag names and attributes as input because at that point you've already lost information. I've been thinking about doing something like this for a while, but I don't have that much time to put in the parser currently.

@tobiastom
Copy link
Author

I'd love to have something generic, but it might not be necessary for this feature.

What my intention was that you just generate the "tag" like you always do, but we have some saying in it before it is stringified. Can you elaborate a little bit about the problems you see with pre blocks? I just don't see any big problems right now. We don't get access to the markdown level, we can just modify the tags and attributes your library generates.

P.S. Having a parsed DOM like structure for markdown was always my dream. Is there already an issue for that?

@michelf
Copy link
Owner

michelf commented May 27, 2015

The problem with code blocks is that it really is two tags. While you could change the attributes on either one with your tag_handler, you'll be missing some contextual information. For instance, how do you know you're in a pre when handleTag receives the code tag name? Beside, some people have requested in the past a way to have a separate tool generate the whole code block, including the <pre><code>, for things such as line numbering.

If we're going to rewrite every HTML-emitting code in PHP Markdown to use a custom handler like above, then we probably ought to do it in a way that does not lock you into emitting small variants of the current HTML structure.

No, genericness is indeed not needed for this feature.

Having a parsed DOM like structure for markdown was always my dream. Is there already an issue for that?

No, I don't think there's an issue for that. Feel free to create one.

@tobiastom
Copy link
Author

Actually, I think the context does not matter. Not because I don't need it for my feature, but more for the following reason: when you want to change something and you need the context, you should use the "DOM" like interface to modify it to your needs. If you just want to add an attribute to all elements of a type, or change a h1 to a h2, you can use this tag_handler.

Both features together would perfectly cover both use cases. I have created #206 for the more advanced feature.

@tobiastom
Copy link
Author

@michelf I just reached the problem with the srcset again and was reminded of this issue. :)

Given that the context would be a different feature for some other time, do you see any problem with the tag_handler itself? I know that it would not solve every problem, but it would solve the concrete example of a srcset, for this issue.

@michelf
Copy link
Owner

michelf commented Oct 15, 2015

Are you proposing a tag_handler that would work only with images? Or are you proposing a tag_handler that will work with all tags emitted by the Markdown parser? Because if it's the later, the difference in the amount of work between this and a more generic solution is pretty negligible. If it's the former, then the name tag_handler is a little bit too generic.

The generic solution I'm thinking about is to have an interface like this:

interface MarkdownComposer {
   function header($content, $level, $attributes);
   function link($content, $href, $attributes);
   function image($src, $attributes);
   function codeBlock($content, $language, $attributes);
   function paragraph($content, $attributes);
   ...
}

and move the current HTML-generating code in those separate functions. Then you could set a custom object to the composer configuration variable if you want to have control over the output.

These functions represents Markdown concepts, not HTML tags, although in many cases they map one on one. Most of the work to get there is to disentangle the HTML-generating code from the rest of the logic, something that needs to be done with the tag_handler approach too.

@tobiastom
Copy link
Author

I love your generic approach!

@tobiastom
Copy link
Author

As I stumbled on retina images again, is there anything I can do to help implement this?

@michelf
Copy link
Owner

michelf commented May 24, 2018

Well, I'm not working on this at the moment. So if you want feel free to go ahead, implement it, and make a pull request. I can answer questions too.

@michael-e
Copy link

I would not vote for this additional feature, simply because srcset is not in the original Markdown specs. (However, I admit that I always post-process HTML with XSLT, so I always have a solution for this issue anyway.)

@tobiastom
Copy link
Author

@michelf I'll try to find some time to look into this in the week after the next one. If I have questions, I'll let you know.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants