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

Ajoute la génération des smileys au format PNG depuis le SVG #6546

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

Arnaud-D
Copy link
Contributor

@Arnaud-D Arnaud-D commented Oct 28, 2023

Génère automatiquement des versions PNG des smileys SVG et utilisables pour les epub.

  • conversion via un script Python utilisant cairosvg (nouvelle dépendance)
  • intégration de ce script dans le pipeline gulp (avec une nouvelle dépendance, gulp-run)
  • possibilité de supplanter les versions générer automatiquement par des png fait main (par exemple pour ninja.png)

Contrôle qualité

  • Vérifier que le front s'installe correctement (make install-front)
  • Vérifier que le front se build correctement (make build-front)
  • Vérifier qu'il n'y a pas de smileys cassés massivement par le processus de conversion (quelques menues différences sont tolérées). Les smileys générés sont dans dist/smileys/png.

@Arnaud-D Arnaud-D added C-Front Concerne l'interface du site hacktoberfest-accepted Pull request approuvée pour le Hacktoberfest labels Oct 28, 2023
@Arnaud-D Arnaud-D added this to En développement in Suivi des PR via automation Oct 28, 2023
@Arnaud-D Arnaud-D moved this from En développement to En attente de QA in Suivi des PR Oct 28, 2023
@Arnaud-D Arnaud-D moved this from En attente de QA to En développement in Suivi des PR Oct 28, 2023
@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Oct 28, 2023

Je n'ai pas configuré correctement la CI... Vous auriez des indications ? J'imagine qu'il faut que cette partie installe ce qu'il faut pour éxécuter un bout de python pour builder le front. Je n'ai jamais fait de mises à jour significatives dans cette partie du code.

Aussi, ça ne devrait pas être trop compliqué de gérer la mise à jour de l'export des epub. J'ai trouvé une ligne qui pourrait suffire.

@Situphen
Copy link
Member

Je n'ai pas configuré correctement la CI... Vous auriez des indications ? J'imagine qu'il faut que cette partie installe ce qu'il faut pour éxécuter un bout de python pour builder le front. Je n'ai jamais fait de mises à jour significatives dans cette partie du code.

Oui, il faudrait à minima rajouter ce bout de code avant l'exécution du make build-front :

      - name: Set up Python ${{ env.PYTHON_VERSION }}
        uses: actions/setup-python@v4
        with:
          python-version: "${{ env.PYTHON_VERSION }}"

      - name: Upgrade pip
        run: pip install --upgrade pip

      - name: Install cairosvg dependency
        run: pip install cairosvg

@coveralls
Copy link

coveralls commented Oct 28, 2023

Coverage Status

coverage: 88.699%. remained the same
when pulling c270972 on Arnaud-D:generation-png-depuis-svg
into 6e0d801 on zestedesavoir:dev.

@Arnaud-D
Copy link
Contributor Author

Super, ça marche maintenant !

@Arnaud-D Arnaud-D moved this from En développement to En attente de QA in Suivi des PR Oct 28, 2023
Copy link
Member

@philippemilink philippemilink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je vois quelques problèmes.

make install-front n'installe pas cairosvg et donc make build-front ne fonctionne pas par la suite. Je ferais un fichier requirements-front.txt avec cairosvg==2.7.1, j'ajouterais la ligne pip install --upgrade -r requirements-front.txt dans la commande make instrall-front et aussi dans la CI (comme ça, on profite de la version de cairosvg qui est figée).

Le fichier convert_smileys_to_svg.py doit s'appeller convert_smileys_to_png.py :) Dans l'en-tête du fichier, ajoute un petit commentaire expliquant à quoi il sert, où il est utilisé, pourquoi on en a besoin, etc...

Est-ce qu'on a encore besoin de garder tous les fichiers PNG qui sont dans assets/simleys/ ?

Aussi, ça ne devrait pas être trop compliqué de gérer la mise à jour de l'export des epub. J'ai trouvé une ligne qui pourrait suffire.

Malheureusement, ça ne suffit pas... Je viens d'essayer, et les sources du ePUB contiennent des <img alt=":-°" class="smiley" src="../images/siffle.svg"/>...
(en SVG et pas de dossier images dans les sources du ePUB).

Je pense qu'on pourrait essayer de régler le problème des ePUBs directement dans cette PR, parce que sinon cette PR convertit des SVGs en PNGs pour rien...

import cairosvg


def convert_folder_to_svg(src_folder: pathlib.Path, dst_folder: pathlib.Path) -> int:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def convert_folder_to_svg(src_folder: pathlib.Path, dst_folder: pathlib.Path) -> int:
def convert_folder_to_png(src_folder: pathlib.Path, dst_folder: pathlib.Path) -> int:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fait.

"""
Convert all SVGs from src_folder into PNGs and write them in dst_folder.
Create dst_folder if needed.
Existing files in dst_fodler are overwritten.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Existing files in dst_fodler are overwritten.
Existing files in dst_folder are overwritten.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fait.

Suivi des PR automation moved this from En attente de QA to Modification demandée Nov 1, 2023
@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Dec 9, 2023

Le fichier convert_smileys_to_svg.py doit s'appeller convert_smileys_to_png.py :)

Fait.

Dans l'en-tête du fichier, ajoute un petit commentaire expliquant à quoi il sert, où il est utilisé, pourquoi on en a besoin, etc...

Fait.

Est-ce qu'on a encore besoin de garder tous les fichiers PNG qui sont dans assets/simleys/ ?

Je ne sais pas où ça serait encore utilisé, malheureusement. Je sais que certains trucs sont en cache (le rendu des messages du forum ?) et que ça peut faire des choses bizarres dans certains cas. Je préfère éviter de toucher et de casser quelque chose que je ne maîtrise pas.

Aussi, ça ne devrait pas être trop compliqué de gérer la mise à jour de l'export des epub. J'ai trouvé une ligne qui pourrait suffire.

Malheureusement, ça ne suffit pas... Je viens d'essayer, et les sources du ePUB contiennent des <img alt=":-°" class="smiley" src="../images/siffle.svg"/>... (en SVG et pas de dossier images dans les sources du ePUB).

Je pense qu'on pourrait essayer de régler le problème des ePUBs directement dans cette PR, parce que sinon cette PR convertit des SVGs en PNGs pour rien...

Je vais regarder ça.

make install-front n'installe pas cairosvg et donc make build-front ne fonctionne pas par la suite. Je ferais un fichier requirements-front.txt avec cairosvg==2.7.1, j'ajouterais la ligne pip install --upgrade -r requirements-front.txt dans la commande make instrall-front et aussi dans la CI (comme ça, on profite de la version de cairosvg qui est figée).

Et ça aussi je vais le faire.

@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Dec 9, 2023

J'ai regardé un peu comment on pourrait aller au bout des choses pour les epubs.

Je pense que cette PR a de la valeur sans aller jusqu'au bout, puisque ça clot une étape dont la suite peut-être faite après coup.

Surtout que les émoticones semblent gérées à cet endroit-là par zmd : https://github.com/zestedesavoir/zmarkdown/blob/e600131bdcb03dcc2e807a023df9baa838873df0/packages/zmarkdown/config/mdast/emoticons.js Du peu que je comprends, ce fichier influence le rendu HTML, donc le rendu sur le site et l'epub à la fois. Je ne sais pas séparer les deux.

@Arnaud-D Arnaud-D moved this from Modification demandée to En attente de retours in Suivi des PR Jan 1, 2024
@Arnaud-D Arnaud-D force-pushed the generation-png-depuis-svg branch 3 times, most recently from 3391d09 to ba67ca2 Compare March 9, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Front Concerne l'interface du site hacktoberfest-accepted Pull request approuvée pour le Hacktoberfest
Projects
Suivi des PR
  
En attente de retours
Development

Successfully merging this pull request may close these issues.

None yet

4 participants