Re: [gentoo-portage-dev] [PATCH] isolated-functions.sh: eliminate loop in has()

[Anthony G. Basile]

On 4/22/16 9:07 AM, rindeal wrote:
>>From edc6df44de4e0f22322062c7c7e1b973bd89f4cd Mon Sep 17 00:00:00 2001
> From: Jan Chren 
> Date: Fri, 22 Apr 2016 14:21:08 +0200
> Subject: [PATCH] isolated-functions.sh: eliminate loop in has()
> 
> Looping is slow and clutters debug log.
> Still this wouldn't matter that much if has() wasn't one of the most used
> functions.

do you have any benchmarks?  what you say makes sense but i'm not sure
of the implementation details of "$A" == "*${B}*" so its hard to say.

> 
> Thus this patch should bring a general improvement.
> ---
>  bin/isolated-functions.sh | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> index e320f71..6900f99 100644
> --- a/bin/isolated-functions.sh
> +++ b/bin/isolated-functions.sh
> @@ -463,14 +463,12 @@ hasv() {
>  }
> 
>  has() {
> -   local needle=$1
> +   local needle=$'\a'"$1"$'\a'

why the ascii bell?  just because you'd never expect it in a parameter
to has?

> shift
> +   local IFS=$'\a'
> +   local haystack=$'\a'"$@"$'\a'

you want "$*" here not "$@"

> 
> -   local x
> -   for x in "$@"; do
> -   [ "${x}" = "${needle}" ] && return 0
> -   done
> -   return 1
> +   [[ "${haystack}" == *"${needle}"* ]]
>  }
> 
>  __repo_attr() {
> --
> 2.7.3
> 


-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197

Re: [gentoo-portage-dev] [PATCH] isolated-functions.sh: eliminate loop in has()

[Zac Medico]

On 04/22/2016 06:07 AM, rindeal wrote:
> From edc6df44de4e0f22322062c7c7e1b973bd89f4cd Mon Sep 17 00:00:00 2001
> From: Jan Chren >
> Date: Fri, 22 Apr 2016 14:21:08 +0200
> Subject: [PATCH] isolated-functions.sh: eliminate loop in has()
> 
> Looping is slow and clutters debug log.
> Still this wouldn't matter that much if has() wasn't one of the most
> used functions.
> 
> Thus this patch should bring a general improvement.
> ---
>  bin/isolated-functions.sh | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> index e320f71..6900f99 100644
> --- a/bin/isolated-functions.sh
> +++ b/bin/isolated-functions.sh
> @@ -463,14 +463,12 @@ hasv() {
>  }
> 
>  has() {
> -   local needle=$1
> +   local needle=$'\a'"$1"$'\a'
> shift
> +   local IFS=$'\a'
> +   local haystack=$'\a'"$@"$'\a'
> 
> -   local x
> -   for x in "$@"; do
> -   [ "${x}" = "${needle}" ] && return 0
> -   done
> -   return 1
> +   [[ "${haystack}" == *"${needle}"* ]]
>  }
> 
>  __repo_attr() {
> --
> 2.7.3

We used to have a similar implementation, but it was changed to a loop
in order to be 100% compatible with PMS. As far as I know, the only way
to implement it in a way that truely respects whitespace in elements is
with a loop.

BTW, your version would have to use this in order to respect word
broundaries:

[[ " ${haystack} " == *" ${needle} "* ]]

However, that still doesn't completely respect whitespace. Consider:

has " b c " a b c d e

I'm not aware of any cases in which we actually need to respect
whitespace in the has function, but according to PMS is should respect
whitespace IIRC.
-- 
Thanks,
Zac