Produits Entr'ouvert » Combo

+    if key.startswith('media:'):

On pourrait imaginer tout ça sous une URL séparée, pour s'épagner ce test.

+            if not asset.is_image():

Pourquoi accepter uniquement les images et pas les autres types de fichiers, ça me semble arbitraire ?

+            root, ext = os.path.splitext(asset.name)

Je ne suis vraiment pas fan d'appeler ça "root" (même si c'est la forme reprise dans la documentation Python), tu pourrais juste zapper la recherche d'un nom convenable,

            if os.path.splitext(asset.name)[0] == ...:

Plutôt faire le key.replace() en-dehors de la boucle. (et ajouter , 1 derrière pour juster virer l'occurence du début).

Aujourd'hui,

    url(r'^assets/(?P<key>[\w_:-]+)$', views.serve_asset),

ça viendrait poser certaines exigences sur les caractères du fichier.

Mais en fait ce ticket c'est 1/ pour ne pas avoir à donner d'extension (mais ça me semble accessoire), 2/ contourner le fait que les fichiers des médias sont uploadés dans des répertoires datés. C'est bien ça ?

J'ai l'impression que oui, et que le test est à la ramasse,

+    Asset(key='media:test', asset=File(BytesIO(b'test'), 'test.png')).save()

qu'il s'agit justement de traiter des fichiers où il n'y a pas d'objet Asset.

Au final c'est peu de code et ça me laisserait aller à accepter (surtout si c'était tapé sur une URL différente), mais quand même, ça me semble une approche totalement ad hoc, sans perspective.

Oui, c'est du dev ad-hoc (...)

Ce qui généralement n'aurait pas sa place ici.

Je posais cette question :

Mais en fait ce ticket c'est 1/ pour ne pas avoir à donner d'extension (mais ça me semble accessoire), 2/ contourner le fait que les fichiers des médias sont uploadés dans des répertoires datés. C'est bien ça ?

Je la posais pour ne pas partir à réfléchir à des changements qui répondraient à ces questions si ce n'étaient pas les bonnes.

_~~

Le patch attaché modifie encore la vue "serve_asset".

Aussi, je ne l'écrivais pas, mais une raison pour passer à une vue particulière serait de dégager la contrainte [\w_-]+, que tu gardes, et qui fait partie du côté inutilement ad hoc du patch.

Pour aller jusqu'au bout, je dirais qu'on veut assets/media-lookup/(?P<filename>tout sauf genre des /+)$'.

Dans def serve_media_asset(request, key): key serait remplacé par filename; et dans la boucle ça matcherait aussi si le nom complet du fichier correspond; dans cette idée :

- def serve_media_asset(request, key):
+ def serve_media_asset(request, filename):
      for asset in CkEditorAsset.get_assets(request):
-         if os.path.splitext(asset.name)[0] == key:
+         if asset.name == filename os.path.splitext(asset.name)[0] == filename:

Ainsi déjà on irait vers quelque chose qu'on pourrait considérer plus généralement utile.

Pourquoi encore exclure certains caractères (autres que le /) ?

Frédéric Péters a écrit :

Pourquoi encore exclure certains caractères (autres que le /) ?

Branche à jour avec uniquement l'exclusion de "/" dans la regex.

C'est une fonctionnalité encore souhaitée ?