On 17/06/10 23:53, Andres P wrote:
On Thu, Jun 17, 2010 at 8:52 AM, Allan McRae<[email protected]>  wrote:
On 17/06/10 22:44, Andres P wrote:

During check_sanity, use regex and abstract the series of variable checks
into
a list.

Signed-off-by: Andres P<[email protected]>
---
  scripts/makepkg.sh.in |   70
+++++++++++++++++++-----------------------------
  1 files changed, 28 insertions(+), 42 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 23e3b36..991ad0f 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1161,6 +1161,19 @@ install_package() {
        fi
  }

+var_lint() {
+       local pattern="$1"
+       local directive="$2"
+       shift 2
+
+       local i
+       for i; do
+               [[ $i =~ $pattern ]] || continue
+               error "$(gettext "'%s' is an invalid value for %s")" "$i"
"$directive"
+               return 1
+       done
+}

I am against this as the error messages are no longer informative.

Allan


Well, the error message would be the least of worries now that it's in
one place instead of>= 7.

What type of error message would be informative?

"variable %s may not match regex %s"

And if makepkg has code repetition because of documentation, then the
man page out to be fixed? Not that the error message is less
descriptive as it is anyhow.

The new error message is much less descriptive; "pkgname is not allowed to start with a hyphen" is very clear, while "'-foo' is an invalid value for pkgname" does not tell me what is wrong.

Using the regex in the error message is just going to confuse the hell out of people too. Reading an error message should not require you know regexes.

And your quest to reduce code duplication can go to the point of being nonsensical. e.g. in this patch:

> +  
> +  local i
> +  for i in 'pkgver' 'pkgrel'; do
> +          var_lint '-' $i "${!i}" || return
> +  done
>

That would be two lines without the loop...


Reply via email to