[PATCH] Allow runpylint.sh to be passed files

[Brian C. Lane]

From: "Brian C. Lane"

---
 tests/pylint/runpylint.sh | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/pylint/runpylint.sh b/tests/pylint/runpylint.sh
index 2cf5797..6fe1b6c 100755
--- a/tests/pylint/runpylint.sh
+++ b/tests/pylint/runpylint.sh
@@ -44,10 +44,11 @@ DISABLED_ERR_OPTIONS="--disable=E1103"
 DISABLED_WARN_OPTIONS="--disable=W0110,W0141,W0142,W0223,W0403,W0511,W0603,W0604,W0613,W0614"

usage () {
-  echo "usage: `basename $0` [--strict] [--help]"
+  echo "usage: `basename $0` [--strict] [--help] [files...]"
   exit $1
 }

+FILES=
 while [ $# -gt 0 ]; do
   case $1 in
     --strict)
@@ -57,8 +58,8 @@ while [ $# -gt 0 ]; do
       usage 0
       ;;
     *)
-      echo "Error unknown option: $1"
-      usage 1
+      FILES=$@
+      break
   esac
   shift
 done
@@ -76,7 +77,10 @@ fi

# run pylint one file / module at a time, otherwise it sometimes gets
 # confused
-for i in "${top_srcdir}"/anaconda $(find "${top_srcdir}/pyanaconda" -type f -name '*py' ! -executable); do
+if [ -z "$FILES" ]; then
+    FILES="${top_srcdir}/anaconda $(find "${top_srcdir}/pyanaconda" -type f -name '*py' ! -executable)"
+fi
+for i in $FILES; do
   if [ -n "$(echo "$i" | grep 'pyanaconda/packaging/dnfpayload.py$')" ]; then
      continue
   fi

[Chris Lumens]

> tests/pylint/runpylint.sh | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Looks good.

- Chris

[David Shea]

On 09/12/2013 05:28 PM, Brian C. Lane wrote:
> From: "Brian C. Lane"
>
> tests/pylint/runpylint.sh | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/tests/pylint/runpylint.sh b/tests/pylint/runpylint.sh
> index 2cf5797..6fe1b6c 100755
> --- a/tests/pylint/runpylint.sh
> +++ b/tests/pylint/runpylint.sh
> @@ -44,10 +44,11 @@ DISABLED_ERR_OPTIONS="--disable=E1103"
>   DISABLED_WARN_OPTIONS="--disable=W0110,W0141,W0142,W0223,W0403,W0511,W0603,W0604,W0613,W0614"
>
> usage () {
> echo "usage: `basename $0` [--strict] [--help]"
> echo "usage: `basename $0` [--strict] [--help] [files...]"
>  exit $1
> }
>
> +FILES=
>   while [ $# -gt 0 ]; do
>     case $1 in
>       --strict)
> @@ -57,8 +58,8 @@ while [ $# -gt 0 ]; do
>         usage 0
>         ;;
>       *)
>  echo "Error unknown option: $1"
>  usage 1
>  FILES=$@
>  break
> esac
>   shift
> done
>
> @@ -76,7 +77,10 @@ fi
>
> # run pylint one file / module at a time, otherwise it sometimes gets
>   # confused
> -for i in "${top_srcdir}"/anaconda $(find "${top_srcdir}/pyanaconda" -type f -name '*py' ! -executable); do
> +if [ -z "$FILES" ]; then
> FILES="${top_srcdir}/anaconda $(find "${top_srcdir}/pyanaconda" -type f -name '*py' ! -executable)"
>
> +fi
> +for i in $FILES; do
>     if [ -n "$(echo "$i" | grep 'pyanaconda/packaging/dnfpayload.py$')" ]; then
>        continue
>     fi

I just have one nitpick, because using $@ like that looks kinda gross ;-)

You can get rid of $FILES entirely and just use the positional 
arguments. Break when an unknown option is hit, and then use something 
like this to loop over the filenames:

if [ $# -eq 0 ]; then
   set -- "${top_srcdir}/anaconda" $(find "${top_srcdir}/pyanaconda" 
-type f -name '*py' ! -executable)
fi
for i in "$@" ; do

and that way at least one use case will be quoted correctly in the loop 
arguments.

[Brian C. Lane]

On Fri, Sep 13, 2013 at 10:57:02AM -0400, David Shea wrote:
> On 09/12/2013 05:28 PM, Brian C. Lane wrote:
> > From: "Brian C. Lane"
> >
> > tests/pylint/runpylint.sh | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/pylint/runpylint.sh b/tests/pylint/runpylint.sh
> > index 2cf5797..6fe1b6c 100755
> > --- a/tests/pylint/runpylint.sh
> > +++ b/tests/pylint/runpylint.sh
> > @@ -44,10 +44,11 @@ DISABLED_ERR_OPTIONS="--disable=E1103"
> >  DISABLED_WARN_OPTIONS="--disable=W0110,W0141,W0142,W0223,W0403,W0511,W0603,W0604,W0613,W0614"
> >  usage () {
> > echo "usage: `basename $0` [--strict] [--help]"
> > echo "usage: `basename $0` [--strict] [--help] [files...]"
> > exit $1
> >
> > }
> > +FILES=
> >  while [ $# -gt 0 ]; do
> >    case $1 in
> >      --strict)
> > @@ -57,8 +58,8 @@ while [ $# -gt 0 ]; do
> >        usage 0
> >        ;;
> >      *)
> >  echo "Error unknown option: $1"
> >  usage 1
> >  FILES=$@
> >  break
> > esac
> >  shift
> >
> > done
> > @@ -76,7 +77,10 @@ fi
> >  # run pylint one file / module at a time, otherwise it sometimes gets
> >  # confused
> > -for i in "${top_srcdir}"/anaconda $(find "${top_srcdir}/pyanaconda" -type f -name '*py' ! -executable); do
> > +if [ -z "$FILES" ]; then
> > FILES="${top_srcdir}/anaconda $(find "${top_srcdir}/pyanaconda" -type f -name '*py' ! -executable)"
> >
> > +fi
> > +for i in $FILES; do
> >    if [ -n "$(echo "$i" | grep 'pyanaconda/packaging/dnfpayload.py$')" ]; then
> >       continue
> >    fi
>
> I just have one nitpick, because using $@ like that looks kinda gross ;-)
>
> You can get rid of $FILES entirely and just use the positional
> arguments. Break when an unknown option is hit, and then use
> something like this to loop over the filenames:
>
> if [ $# -eq 0 ]; then
>   set -- "${top_srcdir}/anaconda" $(find "${top_srcdir}/pyanaconda"
> -type f -name '*py' ! -executable)
> fi
> for i in "$@" ; do
>
> and that way at least one use case will be quoted correctly in the
> loop arguments.

I like my way better :) I prefer to process cmdline stuff in one place,
feed it into variables and then use those everywhere else instead of
scattering bash variables like $# and $@ other places in the code.