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

Allow setting image sizes #218

Merged
merged 14 commits into from May 10, 2016
Merged

Allow setting image sizes #218

merged 14 commits into from May 10, 2016

Conversation

paternal
Copy link
Contributor

@paternal paternal commented May 4, 2016

Closes #83.

@paternal
Copy link
Contributor Author

paternal commented May 6, 2016

  • J'ai finalement opté pour la syntaxe size=2cmx1cm pour une taille absolue, scale=2.0 pour multiplier la taille par deux : permettre size=20%x30% pour autoriser une taille relative différente pour les deux dimensions serait compliqué à convertir en LaTeX.
  • J'ai abandonné la possibilité de définir une taille relative à l'espace disponible (remplir la moitié de la ligne par exemple). On n'arrive pas à se mettre d'accord ; ça sera plus ou moins difficile d'implémenter cela dans les différentes langages de sortie ; ça va rendre le code moins élégant.

Bonne relecture !

@paternal paternal modified the milestones: Version 5, Version 5.1 May 7, 2016
@@ -124,6 +129,10 @@ def search_file(self, filename, extensions=None, *, datadirs=None):
)
return None

@staticmethod
def _render_size(size):
return "TODO"
Copy link
Contributor

Choose a reason for hiding this comment

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

Serais-ce un reste de développement ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Non, un TODO pour #109. Mais c'est mal supposer que le rendu html utilisera aussi ce filtre jinja2. Corrigé : 5f05d8c.

size=
((?P<widthvalue>\d*\.\d+|\d+)(?P<widthunit>%|cm|em|pt))?
x
((?P<heightvalue>\d*\.\d+|\d+)(?P<heightunit>%|cm|em|pt))?
Copy link
Contributor

Choose a reason for hiding this comment

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

Idée spontanée (mais pas forcément réfléchie) : est-ce que le x ne pourrait pas faire partie de la ligne 138 actuelle?

r"""
^
    size=
    ((?P<heightvalue>\d*\.\d+|\d+)(?P<heightunit>%|cm|em|pt))?
    (x(?P<heightvalue>\d*\.\d+|\d+)(?P<heightunit>%|cm|em|pt))?
"""

Ainsi size=3cm est valide (et impose seulement la largeur).


Autre proposition (totalement indépendante) : remplacer le x par *
2cm*3cm à la place de 2cmx3cm

En revanche, si seule la hauteur est personnalisée, ça donne size=*3cm, ce qui pe paraître assez bizarre)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pour le size=3cm, ça ne me parait pas évident que le 3cm désigne la largeur et non pas la hauteur. C'est pour ça que j'ai imposé le size=3cmx, qui n'est pas ambigü. Mais si tu préfères, le x peut être optionnel : avec ma pataretraite qui est proche, vu que c'est vous qui allez gérer, autant faire comme vous le souhaitez.

Pour remplacer le x par *, c'est possible, on peut aussi autoriser les deux. Comme tu veux.

Slight behoavior differences between all() and != (None,None):
- all() requires ALL terms to be not None (where (None, "cm") would pass
  != (None,None))
- all() checks for None or 0 or empty string
	No empty unit is supported (yet)
	A size of 0 does not really makes sense (and currently is passed
since it is the string "0")
@oliverpool
Copy link
Contributor

(@paternal : je me suis permis un léger changement, en espérant être plus pythonique, n'hésite pas à le défaire ou l'améliorer !)

@oliverpool
Copy link
Contributor

oliverpool commented May 8, 2016

Pour répondre à la plupart de mes suggestions, on pourrait faire en chordpro quelque chose de plus proche du LaTeX:

{image: file.png width=15cm height=5px} # '%' is not supported for height or width

Et autoriser aussi {image: file.png scale=.2} (mais pas les deux)

Du coup il n'y a plus de x (ou de *) et on garde le scale tel quel.

@paternal si ça te convient, je peux prendre le temps de proposer une implémentation

@paternal
Copy link
Contributor Author

paternal commented May 9, 2016

@oliverpool : Tu as raison : c'est plus clair ainsi. f4e1243

@oliverpool
Copy link
Contributor

Ca me va !
Je me suis permis quelques petites retouches des messages d'erreurs, ainsi qu'un essai d'être plus pythonique.

@paternal paternal merged commit 17dc125 into master May 10, 2016
@paternal paternal deleted the image_size branch May 10, 2016 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants