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

Mise a jour markdown #2291

Merged
merged 9 commits into from
Feb 24, 2015
Merged

Mise a jour markdown #2291

merged 9 commits into from
Feb 24, 2015

Conversation

cgabard
Copy link
Contributor

@cgabard cgabard commented Feb 11, 2015

Q R
Correction de bugs ? oui
Nouvelle Fonctionnalité ? non
Tickets (issues) concernés voir en dessous

Bonne mise a jour de markdown. Quelques infos pour guider la QA et lister les mises à jour :

Je n'ai pas eu le temps de faire des tests super poussés donc je ne garranti pas à 100%. Je PR tout de même pour que Situphen puisse ajouter les CSS et pour commencer la QA car je n'aurais pas le net avant Lundi, si il y a des problèmes, je pourrais par contre peut etre les corriger dans le week end.

@cgabard
Copy link
Contributor Author

cgabard commented Feb 11, 2015

raaaaa foutu unicode

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling bd4c6ca on cgabard:update_markdown into c490264 on zestedesavoir:dev.

@Situphen
Copy link
Member

\o/ Je te fais une PR ! ;)

@Eskimon
Copy link
Contributor

Eskimon commented Feb 12, 2015

Eh beh Travis est vraiment pas content : FAILED (failures=3, errors=19)

@firm1
Copy link
Contributor

firm1 commented Feb 12, 2015

Les erreurs semblent venir de jsfiddle.

@Eskimon
Copy link
Contributor

Eskimon commented Feb 12, 2015

J'ai plutot l'impression que c'est le filtre emarkdown qui met tout en vrac.

Christophe Gabard added 2 commits February 16, 2015 19:07
Rendre les tableaux et les formules MathJax responsives
@cgabard
Copy link
Contributor Author

cgabard commented Feb 16, 2015

Je suis pas sûrs de comprendre les erreurs de Travis, je comprend pas trop ce que j'aurais put faire pour générer ça...

@cgabard
Copy link
Contributor Author

cgabard commented Feb 17, 2015

Quelqu'un peut m'expliquer tous ces problème de font de pandoc ?

@Eskimon
Copy link
Contributor

Eskimon commented Feb 17, 2015

Idem "WTF flake8" ?

@cgabard
Copy link
Contributor Author

cgabard commented Feb 17, 2015

Bon manifestement toutes les PR en cours se font jeter a cause de ça.

Un des tests semble planter ici : https://travis-ci.org/zestedesavoir/zds-site/jobs/50974951

FAIL: test_emarkdown (zds.utils.templatetags.tests.test_emarkdown.EMarkdownTest)

----------------------------------------------------------------------

Traceback (most recent call last):

File "/home/travis/build/zestedesavoir/zds-site/zds/utils/templatetags/tests/test_emarkdown.py", line 22, in test_emarkdown

"</blockquote>", tr)

AssertionError: u'<h3>Titre 1</h3>\n<h4>Titre <strong>2</strong></h4>\n<h5>Titre 3</h5>\n<blockquote>\n<p>test</p>\n</blockquote>' != u'<h1>Titre 1</h1>\n<h2>Titre <strong>2</strong></h2>\n<h3>Titre 3</h3>\n<blockquote>\n<p>test</p>\n</blockquote>'

Ce qui m'étonne, et bien que c'est moi qui ai ajouté ce test, c'est qu'il n'a aucun sens en réalité. Je comprend pas trop pourquoi on test que le rendu html comporte bien les titres décalés alors que ce n'est pas demandé dans le filtre... J'ai rien changé dans le markdown à ce propos et en réalité ça m'étonne surtout que ça marchait avant ça !

@cgabard
Copy link
Contributor Author

cgabard commented Feb 17, 2015

Bon j'ai rien dis, j'avais oublié qu'il y avait bien un décalage des titres fait automatiquement par le zmarkdown. Et c'est visiblement cassé...

@cgabard
Copy link
Contributor Author

cgabard commented Feb 17, 2015

bon j'ai re-mergé avec dev, on va espérer que les problemes de flake sont réglé

@cgabard
Copy link
Contributor Author

cgabard commented Feb 17, 2015

Allons bon, maintenant ce sont les tests front qui sont cassé... C'est normal ?

Sinon je pense que c'est bon pour la QA

@Situphen
Copy link
Member

Non c'est pas normal mais ce n'est pas causé par mes modifications, c'est l'installation d'un paquet qui a échoué !

@cgabard
Copy link
Contributor Author

cgabard commented Feb 17, 2015

quelqu'un pour relancer le test du coup ? poke @firm1 ou @SpaceFox

@cgabard
Copy link
Contributor Author

cgabard commented Feb 18, 2015

Bon donc ça passe, c'est au tour de la QA maintenant :)

@Situphen
Copy link
Member

Fonctionnalité Etat
Bloc de code OK (testé : 2 différentes syntaxes Markdown avec du code Python)
Compatibilité Python 3 Je ne sais pas comment tester ça
Formules et tableaux OK, il faudra juste mettre à jour le cache lors de la MEP
Trim des espaces dans les codes inlines Il y a bien les espaces de présent au niveau de l'HTML, mais rien ne change visuellement
URL relative OK
URL avec : dedans OK
KBD dans une liste OK
HTTPS forcé pour jsfiddle Je ne sais pas comment tester ça

@Eskimon
Copy link
Contributor

Eskimon commented Feb 18, 2015

HTTPS forcé pour jsfiddle

C'est un truc un peu chiant, il faut activer le support jsfiddle dans un article/tuto et ensuite tu balances un lien vers un filddle. Ce dernier devra être transforme pour passer en https

@Situphen
Copy link
Member

JSFiddle ne fonctionne pas et j'ai ça dans la console (poke @cgabard) :

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/extensions/__init__.py:37: DeprecationWarning: Extension classes accepting positional args is pending Deprecation. Each setting should be passed into the Class as a keyword. Positional args are deprecated and will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  DeprecationWarning)

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/__init__.py:248: DeprecationWarning: Using short names for Markdown's builtin extensions is deprecated. Use the full path to the extension with Python's dot notation (eg: "markdown.extensions.abbr" instead of "abbr"). The current behavior will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  DeprecationWarning)

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/__init__.py:248: DeprecationWarning: Using short names for Markdown's builtin extensions is deprecated. Use the full path to the extension with Python's dot notation (eg: "markdown.extensions.footnotes" instead of "footnotes"). The current behavior will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  DeprecationWarning)

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/__init__.py:248: DeprecationWarning: Using short names for Markdown's builtin extensions is deprecated. Use the full path to the extension with Python's dot notation (eg: "markdown.extensions.tables" instead of "tables"). The current behavior will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  DeprecationWarning)

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/__init__.py:248: DeprecationWarning: Using short names for Markdown's builtin extensions is deprecated. Use the full path to the extension with Python's dot notation (eg: "markdown.extensions.fenced_code" instead of "fenced_code"). The current behavior will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  DeprecationWarning)

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/__init__.py:211: DeprecationWarning: Setting configs in the Named Extension string is deprecated. It is recommended that you pass an instance of the extension class to Markdown or use the "extension_configs" keyword. The current behavior will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  '2.6 for more info.', DeprecationWarning)

/home/situphen/Documents/Dev/Python/zdsenv/local/lib/python2.7/site-packages/markdown/__init__.py:248: DeprecationWarning: Using short names for Markdown's builtin extensions is deprecated. Use the full path to the extension with Python's dot notation (eg: "markdown.extensions.codehilite" instead of "codehilite"). The current behavior will raise an error in version 2.7. See the Release Notes for Python-Markdown version 2.6 for more info.
  DeprecationWarning)

[<markdown.extensions.subsuperscript.SubSuperscriptExtension object at 0x7f1538c87790>, <markdown.extensions.delext.DelExtension object at 0x7f1538c877d0>, <markdown.extensions.urlize.UrlizeExtension object at 0x7f1538c87810>, <markdown.extensions.smarty.SmartyExtension object at 0x7f1538c87850>, u'abbr', u'footnotes', u'tables', u'fenced_code', u'codehilite(linenums=True,guess_lang=False)', <markdown.extensions.customblock.CustomBlockExtension object at 0x7f1538c87950>, <markdown.extensions.kbd.KbdExtension object at 0x7f1538c878d0>, <markdown.extensions.emoticons.EmoticonExtension object at 0x7f1538c87910>, <markdown.extensions.video.VideoExtension object at 0x7f1538c879d0>, <markdown.extensions.preprocessblock.PreprocessBlockExtension object at 0x7f1538c87a10>, <markdown.extensions.grid_tables.GridTableExtension object at 0x7f1538c87a50>, <markdown.extensions.comments.CommentsExtension object at 0x7f1538c87a90>, <markdown.extensions.smartLegend.SmartLegendExtension object at 0x7f1538c87ad0>, <markdown.extensions.align.AlignExtension object at 0x7f1538c87990>, <markdown.extensions.headerDec.DownHeaderExtension object at 0x7f1538c87b10>, <markdown.extensions.mathjax.MathJaxExtension object at 0x7f1538c87890>]

@Eskimon
Copy link
Contributor

Eskimon commented Feb 18, 2015

ne fonctionne pas

C'est pas un rapport valide ça ! :D Tu as fais quoi, comment et obtenu quoi ? (500 j'ai l'impression ?)

@firm1
Copy link
Contributor

firm1 commented Feb 18, 2015

VIsiblement travis balance pas mal de warning, il faudrait voir pourquoi.

@Situphen
Copy link
Member

Je viens de tester avec le même lien JSFiddle qu'Eskimon avait utiliser lors du développement de la fonctionnalité (!(http://jsfiddle.net/tsoacfub/1/)) et ça fonctionne (en HTTPS comme demandé). [Par contre, il faut que les URLs utilisées dans le script JS du JSFiddle (ici, la connexion à Google Maps) soit en HTTPS sinon on se tape une erreur Cross-Domain du navigateur Firefox.]

J'ai testé avec un autre JSFiddle (http://jsfiddle.net/99cjckyh/) et ça ne fonctionne pas car il n'a pas le /1/ à la fin. Or, si j'essaye d'y accéder avec le /1/, j'obtiens une erreur !

@cgabard
Copy link
Contributor Author

cgabard commented Feb 19, 2015

Bon alors les warning c'est "normal", c'est python markdown qui on changé
un truc qui les génère. Je vais voir pour les virer mais c'est pas
critique.

Pour jsfiddle la révision dans l'URL (les /1/ ou autre chiffre) sont
obligatoire pour assurer que l'utilisateur ne va pas modifier son script
hors validation.

Par contre pour les erreurs cross domaine, je ne sais pas trop ce que je
peux faire du coup.

Christophe
Le 18 févr. 2015 18:02, "Situphen" [email protected] a écrit :

Je viens de tester avec le même lien JSFiddle qu'Eskimon avait utiliser
lors du développement de la fonctionnalité (!(
http://jsfiddle.net/tsoacfub/1/)) et ça fonctionne (en HTTPS comme
demandé). [Par contre, il faut que les URLs utilisées dans le script JS du
JSFiddle (ici, la connexion à Google Maps) soit en HTTPS sinon on se tape
une erreur Cross-Domain du navigateur Firefox.]

J'ai testé avec un autre JSFiddle (http://jsfiddle.net/99cjckyh/) et ça
ne fonctionne pas car il n'a pas le /1/ à la fin. Or, si j'essaye d'y
accéder avec le /1/, j'obtiens une erreur !


Reply to this email directly or view it on GitHub
#2291 (comment)
.

@Situphen
Copy link
Member

Par contre pour les erreurs cross domaine, je ne sais pas trop ce que je peux faire du coup.

Tu ne peux rien faire. De toute façon vu que c'est HTTPS par défaut le script ne fonctionnera pas s'il y a de l'HTTP et non de l'HTTPS et ça en passera pas la validation !

@cgabard
Copy link
Contributor Author

cgabard commented Feb 19, 2015

Ha je viens de comprendre... Ce sont les liens dans le jsfiddle (aux quels
je n'ai pas accès) qui posent problèmes ?

Si oui en effet il faudra prévenir les valido de ça

Christophe
Le 19 févr. 2015 09:39, "Situphen" [email protected] a écrit :

Par contre pour les erreurs cross domaine, je ne sais pas trop ce que je
peux faire du coup.

Tu ne peux rien faire. De toute façon vu que c'est HTTPS par défaut le
script ne fonctionnera pas s'il y a de l'HTTP et non de l'HTTPS et ça en
passera pas la validation !


Reply to this email directly or view it on GitHub
#2291 (comment)
.

@Eskimon
Copy link
Contributor

Eskimon commented Feb 24, 2015

Du coup tout est bon non ? (je confirme le coté OK pour jsfiddle. Si on appel des scripts externe en https ca a l'air Ok)

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

Bon cette PR fait pas mal de chose. Ok le bug des espaces dans les mini-codes est toujours présent. Je ne vais donc pas le fermer coté markdown et je re-regarderais la prochaine fois. Pour les warnings, est ce bloquant ?

@firm1
Copy link
Contributor

firm1 commented Feb 24, 2015

Pour les warnings, est ce bloquant ?

Il y'en a quand même un paquet, du coup si demain on a de vrais warninbg, ça sera chaud de les remarquer.

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

ok je vais essayer de voir ça...

@Eskimon
Copy link
Contributor

Eskimon commented Feb 24, 2015

Surtout que ca ca fait peur :

The current behavior will raise an error in version 2.7

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

The current behavior will raise an error in version 2.7

Oui enfin c'est moi qui controle le merge avec l'upstream...

@Eskimon
Copy link
Contributor

Eskimon commented Feb 24, 2015

On dirait qu'il y a des print ou des affichages de debug dans le code genre ca (c'est pas un warning ou une erreur) : https://travis-ci.org/zestedesavoir/zds-site/jobs/51125011#L1731

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

Oui j'avais laissé trainé un print durant le debug.

Bref je l'ai viré et normalement j'ai dut réduire les warnings.

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

Ok là normalement j'ai corrigé le dernier warning.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8f53d82 on cgabard:update_markdown into * on zestedesavoir:dev*.

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

Quelqu'un pour relancer le build, planté sur une erreur de connexion ssl ? poke @firm1

@cgabard
Copy link
Contributor Author

cgabard commented Feb 24, 2015

QA ok (sauf le coup des mini codes que je garde ouvert dans python-zmarkdown), les tests passent et a priori plus de warning. C'est tout bon pour le merge ?

@Eskimon
Copy link
Contributor

Eskimon commented Feb 24, 2015

Perso je suis partisan du merge avec surveillance en release v1.7

@SpaceFox
Copy link
Contributor

100% d'accord.

SpaceFox added a commit that referenced this pull request Feb 24, 2015
@SpaceFox SpaceFox merged commit 28164ea into zestedesavoir:dev Feb 24, 2015
@SpaceFox SpaceFox added this to the Version 1.7 milestone Feb 24, 2015
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

Successfully merging this pull request may close these issues.

8 participants