nightingale-media-player / nightingale-addons

Add-ons for Nightingale
6 stars 4 forks source link

make.py: fix trailing slash problem #21

Open darealshinji opened 10 years ago

rsjtdrjgfuzkfg commented 10 years ago

Hm. From scrolling through this seems not to work if somebody uses a path for the folder (so you have multiple "/" in the path); I don't know python enough to be sure about this, though.

darealshinji commented 10 years ago

It seems to work, in a weird way though:

djcj ~ $ ./make.py Downloads/nightingale-addons-master/equalizerpane/
Downloadsnightingale-addons-masterequalizerpane-1.0.11.xpi could not be written.
djcj ~ $ ls *.xpi
Downloadsnightingale-addons-masterequalizerpane-1.0.11.xpi
djcj ~ $ 
rsjtdrjgfuzkfg commented 10 years ago

I wouldn't call it working. It is what I expected :P

freaktechnik commented 10 years ago

Couldn't you use a simple regular expression with something like .sub("/$", "")?

darealshinji commented 10 years ago

I'll see what I can do, but to be honest I don't know too much about writing python scripts.

edit: What about using a shell script instead?

#!/bin/sh

if [ -z "$1" ] ; then
  echo "usage: $0 <path>"
  exit 1
fi

version=$(grep -e "em:version" "$1"/install.rdf | cut -d '>' -f2 | cut -d '<' -f1)
xpiName=$(echo $(basename "$1")-$version.xpi)
basedir=$(pwd)

cd "$1" && zip -r "$basedir/$xpiName" ./*

if [ -f "$basedir/$xpiName" ] ; then
  echo "'$basedir/$xpiName' successfully written."
  exit 0
else
  echo "'$basedir/$xpiName' could not be written."
  exit 1
fi
clokep commented 10 years ago

The regular expression that kills the trailing / only seems like the best way to go.

darealshinji commented 10 years ago

I've got it. Now it only removes the trailing slash and make.py can be run from other directories.

darealshinji commented 9 years ago

So, do you want this fix or should I close this PR?

rsjtdrjgfuzkfg commented 9 years ago

Looks fine to me, although I didn't test it. Imho some notes about where to find the built file (after the patch: in the working directory if I read this right) would be fine in the readme as well, but that should not oppose a merge.