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

img wrapped in p #180

Closed
fxck opened this issue Jun 1, 2014 · 21 comments
Closed

img wrapped in p #180

fxck opened this issue Jun 1, 2014 · 21 comments

Comments

@fxck
Copy link

fxck commented Jun 1, 2014

is there a way of disabling wrapping of certain elements(like images) with paragraph?

![](image.jpg)

ends up as

<p><img src="image.jpg" /></p>

I'd like it to be just

<img src="image.jpg" />

I tried changing few things around in the code, but without much of a success..

@kminchev
Copy link

kminchev commented Jun 1, 2014

@fxck
Copy link
Author

fxck commented Jun 1, 2014

..okay?

suggesting to use ->line() or something? I still want the wrapping to work everywhere, just not on images..

from

# heading

paragraph 

![](img.jpg)

paragraph

to

<h1>heading</h1>
<p>paragraph</p>
<img src="img.jpg" />
<p>paragraph</p>

@kminchev
Copy link

kminchev commented Jun 1, 2014

So use line() everywhere.

@fxck
Copy link
Author

fxck commented Jun 1, 2014

What do you mean everywhere, like parsing the text myself into lines and then applying parsedown on each one of those lines. Seriously?

@kminchev
Copy link

kminchev commented Jun 1, 2014

Oh, now I understand what you mean, sorry. It's not possible at the moment and frankly I don't see the need for it. You can parse all of your markup with text() and images with line() or you can use plain HTML inside your markdown.

@fxck
Copy link
Author

fxck commented Jun 1, 2014

Well I do, there's nothing unsemantic about <img> outside of a <p>, last time I checked anyway. And even if it was, the use case is obvious - http://jsbin.com/civik/1/edit?output
With image wrapped with paragraph, image's width's gonna be the width of the paragraph..

Also what you are suggesting would again mean parsing the text myself, find all the pre-image and post-image parts, apply text() to them, apply line to image parts.. now considering there can be 10s of images in an article, that seems like a lot of unnecessarily hassle.

There obviously are elements(block ones) that are not getting wrapped with paragraphs, which means there should be a way to get image to parse the same way..

@apfelbox
Copy link
Contributor

apfelbox commented Jun 1, 2014

@fxck you have a valid use case, but parsedown is just working according to the spec here. I don't think that you can get this change into core.

You have two options though:

  • post processing just transform your text into markdown using the regular ->text() method. Afterwards walk through the generated HTML structure, search for p > img:only-child (or something like that) and replace it with just the <img> tag.
  • custom extension: I am not familiar enough with the internals to tell to the line where you have to look, but you can extend parsedown and just overwrite the proper method to replace the structure while doing the parsing / code generation.

@fxck
Copy link
Author

fxck commented Jun 1, 2014

I'm absolutely fine with someone who's familiar with the code just pointing me in the right direction(as of what to change), I tried changing stuff around myself, but without much of a success.

As for postproccessing, I could probably use this but I'd be happier if I could do it with parsedown directly..

@erusev
Copy link
Owner

erusev commented Jun 1, 2014

I'm absolutely fine with someone who's familiar with the code just pointing me in the right direction

You should override (or change) the buildParagraph method to check whether $Line['text'] holds an image definition and if it does, return the desired HTML output as a string.

@apfelbox
Copy link
Contributor

apfelbox commented Jun 1, 2014

@erusev
Copy link
Owner

erusev commented Jun 1, 2014

@apfelbox Nice!

@fxck
Copy link
Author

fxck commented Jun 1, 2014

@apfelbox looks good, thanks for saving me some time!

@fxck fxck closed this as completed Jun 1, 2014
@fxck
Copy link
Author

fxck commented Jan 13, 2015

@erusev has anything changed in recent versions of parsedown? because what @apfelbox posted doesn't really work anymore(buildParagraph renamed perhaps)?

also parsedown is wrapping custom elements, be it angular directives or web components in <p>, which is in most cases undesirable, I still stand by that there could/should be an option to define which elements should be wrapped in paragraph and which should not..

@erusev
Copy link
Owner

erusev commented Jan 13, 2015

@fxck

Yes, a few things.

The buildParagraph method is called just paragraph in the latest version.

In the @apfelbox post, you should also change $this->identifyLink($Line); to $this->inlineLink($Line['text']);.

@fxck
Copy link
Author

fxck commented Jan 13, 2015

Argument 1 passed to Parsedown::element() must be of the type array, null given

when either $this->inlineLink($Line['text']); or $this->inlineLink($Line); used

@fxck
Copy link
Author

fxck commented Jan 13, 2015

$this->inlineImage() works

@erusev
Copy link
Owner

erusev commented Jan 13, 2015

@fxck

Right.

It'd be great if you could update the gist.

@fxck
Copy link
Author

fxck commented Jan 13, 2015

Can't really, it's @apfelbox's here a new one though.

https://gist.github.com/fxck/d65255218de3611df3cd

@erusev
Copy link
Owner

erusev commented Jan 13, 2015

@fxck

Great, thanks.

@apfelbox
Copy link
Contributor

(I updated my gist to reflect the latest changes)

@kantoniak
Copy link

Updated the gist to work with 1.7.1 and wrap images in <figure> instead of leaving them as-is:
https://gist.github.com/kantoniak/b1a5c7889e5583824487dc78d93da7cd

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

5 participants