On 17/06/10 23:53, Andres P wrote:
On Thu, Jun 17, 2010 at 8:52 AM, Allan McRae<allan@archlinux.org> 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<aepd87@gmail.com> --- 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...