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

Where to go from here? #15

Open
michael-e opened this issue Apr 5, 2018 · 30 comments
Open

Where to go from here? #15

michael-e opened this issue Apr 5, 2018 · 30 comments

Comments

@michael-e
Copy link
Member

Where shall this extension go from here? Meanwhile PHP Markdown has released version 1.8.0, and the current version of HTML Purifier is 4.10.0. So we may consider one of the following:

  1. Update the libraries and keep the rest "as is".
  2. Update the libraries and switch to composer.
  3. Update PHP Markdown and remove HTML Purifier (has it ever been used by anybody?) – that's a breaking change, so it means a 2.0 release.

The update of PHP Markdown would solve #8 .

Regarding the third option: This has also been discussed in #6.

@michael-e
Copy link
Member Author

Oh, there is an additional option as well: One might create a new extension called "PHP Markdown" which implements PHP Markdown only. (I may do this for my personal use anyway, and in this case I might as well release it.)

@nitriques
Copy link
Member

I would vote for option 2). I did used the purifier once on imported content, which was nice.
I would keep all 4 classes.

cc @fhamon

@michael-e
Copy link
Member Author

Oh.

I am not a fan of HTML Purifier, simply because it's so big, much bigger than Symphony.

In this case I am not sure if I'd like to care for it. I will have to think about it. Today I spent some time to write the aforementioned very simple extension that does PHP Markdown, and nothing else. I haven't released it to the public, because I wouldn't like to compete with Symphony's Markdown extension. But IMHO the Markdown extension is too big and tries too much.

(I don't like Smarty Pants either, partly related to the fact that typographically correct quotes are different in German, but also because – reading the docs – Smarty Pants creates new, different problems.)

@nitriques
Copy link
Member

So the option would be to keep 3 classes in Markdown, and create a new one with the purifier, that depends on the first one being installed ? I would live with that... But it needs time which, for now, goes elsewhere (for me). If you want to send PRs and create new repos, I am ok with that !

@michael-e
Copy link
Member Author

At the moment I don't know if any work on "chained" text formatters has been already done. I am afraid that this still is a bigger undertaking.

But, honestly, should we really insist on "backwards compatibility forever" here? We no longer demand that for Symphony, so why for this extension?

The Markdown extension must do Markdown (and Markdown Extra, probably). Should it provide Smarty Pants (which only works in English, has side effects, and has nothing to do with Markdown)? Or even HTML purification? We might think that the "universal" answer depends on the possibility to chain text formatters. Or maybe we should just dare to say that Symphony and core extensions keep things simple? For me the solution actually is simple, as posted above — I have created an created an extension that fits my needs (Markdown and Markdown Extra only).

If chained text formatters are not possible (which I suppose), I can not propose any universal solution, unfortunately.

Please feel free to close this issue if you think that it leads to nowhere. (And I am afraid that the latter is the case if we don't dare to break compatibility in order to simplify things.)

@nitriques
Copy link
Member

I don't know if any work on "chained" text formatters has been already done.

No need to chain them. But "require()" files (link the markdown lib) from the other extension

But, honestly, should we really insist on "backwards compatibility forever" here? We no longer demand that for Symphony, so why for this extension?

I do not !

The thing is, I would like to not loose features. But smaller bundles are great.

PHP Markdown includes normal and extra, and the other two could live in other repos. you would first need to install the "core markdown" to make the other one works (just because they require files, they do not need to chain)

@michael-e
Copy link
Member Author

michael-e commented Apr 15, 2018

Oh, I understand your idea. So we might have three new extensions, allowing lots of text formatter combinations:

  • Extension "PHP Markdown" provides the following text formatters:
    • PHP Markdown
    • PHP Markdown Extra
  • Extension "PHP SmartyPants" provides:
    • PHP SmartyPants
    • PHP Markdown + PHP SmartyPants
    • PHP Markdown + PHP SmartyPants w/ Typographer
    • PHP Markdown Extra + PHP SmartyPants
    • PHP Markdown Extra + PHP SmartyPants w/ Typographer
  • Extension "HTML Purifier" provides:
    • HTML Purifier
    • PHP Markdown + HTML Purifier
    • PHP Markdown + PHP SmartyPants + HTML Purifier
    • PHP Markdown + PHP SmartyPants w/ Typographer + HTML Purifier
    • PHP Markdown Extra + HTML Purifier
    • PHP Markdown Extra + PHP SmartyPants + HTML Purifier
    • PHP Markdown Extra + PHP SmartyPants w/ Typographer + HTML Purifier

Number 2 + 3 would be supposed to check for other extensions in order to see which text formatters can be provided. So, for example, of you install PHP SmartyPants without having PHP Markdown istalled, you will only see the PHP SmartyPants formatter. Right?

There's even more interesting work that might be added to this idea (as a fourth extension) to provide useful "German SmartyPants".

So, for example, if you install all 4 extensions from this "extension family", you would see the following text formatters (sorted alphabetically):

  • HTML Purifier
  • PHP Markdown
  • PHP Markdown + HTML Purifier
  • PHP Markdown + PHP SmartyPants
  • PHP Markdown + PHP SmartyPants (German)
  • PHP Markdown + PHP SmartyPants (German) + HTML Purifier
  • PHP Markdown + PHP SmartyPants (German) w/ Typographer
  • PHP Markdown + PHP SmartyPants (German) w/ Typographer + HTML Purifier
  • PHP Markdown + PHP SmartyPants + HTML Purifier
  • PHP Markdown + PHP SmartyPants w/ Typographer
  • PHP Markdown + PHP SmartyPants w/ Typographer + HTML Purifier
  • PHP Markdown Extra
  • PHP Markdown Extra + HTML Purifier
  • PHP Markdown Extra + PHP SmartyPants
  • PHP Markdown Extra + PHP SmartyPants (German)
  • PHP Markdown Extra + PHP SmartyPants (German) + HTML Purifier
  • PHP Markdown Extra + PHP SmartyPants (German) w/ Typographer
  • PHP Markdown Extra + PHP SmartyPants (German) w/ Typographer + HTML Purifier
  • PHP Markdown Extra + PHP SmartyPants + HTML Purifier
  • PHP Markdown Extra + PHP SmartyPants w/ Typographer
  • PHP Markdown Extra + PHP SmartyPants w/ Typographer + HTML Purifier
  • PHP SmartyPants
  • PHP SmartyPants (German)

Well, that's pretty interesting, conceptually. But here are the downsides, IMHO:

  • The developer must understand the concept of, well, let's call it a "text formatter extension family".
  • The text formatter list in the backend may be overwhelming (because it's supposed to provide all possible combinations).
  • This concept needs a lot of hard-coded "dependency checking" in the extension code.

Honestly, chained text formatters would be a much better solution. And standalone "full-blown" solutions, on the other hand, are much simpler. Experienced Symphony developers can easily create their own private extension(s), providing dedicated text formatter combinations only (instead of an exhaustive list like the one above).

And: This would mean a bit of work… I am afraid that currently neither you nor me find the time for this.

So let's keep this issue open for the moment. There's no pressure here.

(I can release my "minimal extension" now or later, as you like. I can as well keep it private, in order to not create confusion.)

@nitriques
Copy link
Member

you will only see the PHP SmartyPants formatter. Right?

Yes, in an ideal world !

Honestly, chained text formatters would be a much better solution.

For the CMS developer yes, but you would need a UI do create those ?

I can release my "minimal extension" now or later

Yes, please do. Feel free to put it in symphonists org.

@michael-e
Copy link
Member Author

For the CMS developer yes, but you would need a UI do create those ?

Yep. Which is complicated.

Yes, please do. Feel free to put it in symphonists org.

For the moment, I will release the minimal "PHP Markdown" as my own extension, which means that I will actually care for it. :-)

@michael-e
Copy link
Member Author

@nitriques
Copy link
Member

nitriques commented Apr 26, 2018

Crowd goes wild ! Wanna keep it under your name ? Want to add it on sym ext ?

@nitriques
Copy link
Member

Yep. Which is complicated.

Even more than that.

@michael-e
Copy link
Member Author

Want to add it on sym ext ?

Oh, I tried, but sym ext wanted write access to my account. :-(

I posted a message about it on Gitter!

@nitriques
Copy link
Member

Oh, I tried, but sym ext wanted write access to my account. :-(

For real ? That may be my fault...

@michael-e
Copy link
Member Author

Yes, for real!

@nitriques
Copy link
Member

Looks like it is not me ! It's a problem with our scopes works with the github api:
jollygoodcode/jollygoodcode.github.io#6
https://developer.github.com/apps/building-oauth-apps/scopes-for-oauth-apps/

@michael-e
Copy link
Member Author

That's about repos, isn't it? For users, schouldn't it be possible to use the read:user scope?

@michael-e
Copy link
Member Author

My concern is access to user data. When trying to login, it says:

Personal user data
Full access

This application will be able to read and write all user data. This includes the following:

  • Private email addresses
  • Private profile information
  • Followers

@nitriques
Copy link
Member

Let's try it.

@michael-e
Copy link
Member Author

Ah, oh, much better:

Personal user data
Profile information (read-only)

This application will be able to read your private profile information.

And it works: http://symphonyextensions.com/extensions/php_markdown/

@nitriques
Copy link
Member

Can you try again ?

@nitriques
Copy link
Member

Yeah !! 🙌

@michael-e
Copy link
Member Author

michael-e commented Apr 27, 2018

But sym ext says that "Symphony compatibility information for this extension is not available." Which ain't true. Or do you see any mistake in my XML?

@nitriques
Copy link
Member

It's a cron job that creates that... just wait ;) (there are no way of knowing if there is nothing or it is processing...)

@nitriques
Copy link
Member

I've run the cron, but know we need to let the cache expire ... ;)

@nitriques
Copy link
Member

If there was any error, the add would have failed.

@michael-e
Copy link
Member Author

OK, I'll be AFK for an hour or two anyway.

Thanks for fixing the permission thingie!

@nitriques
Copy link
Member

Thanks for fixing the permission thingie!

No problem! Thanks for telling me, I was not even aware of it. Also, I do not want to write to anybody's account ;)

@michael-e
Copy link
Member Author

Ah, bingo! Compat info is there!

@nitriques
Copy link
Member

:D

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

2 participants