On 10/01/14 06:20, Pierre Neidhardt wrote:
> For now I've kept the original pacsearch colors for the repos. Some fields 
> have
> the same colors--e.g. 'extra' and 'version' are both bold green. Maybe it 
> would
> look nicer to make repo/name bold and to leave the other fields regular.  I 
> have
> changed repo-specific variables to formatting variables.  I have been using
> correct indentation rules this time.
> 
> Additionally I have rephrased the help message; the previous message would let
> one think this script was rather useless compared to a simple pacman -Ss.
> 

These patches are getting near done.  I'll have a few comments to make
that will extend over all three patches.

> Signed-off-by: Pierre Neidhardt <[email protected]>
> ---
>  contrib/pacsearch.in | 55 
> +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in
> index 624f201..2418e19 100644
> --- a/contrib/pacsearch.in
> +++ b/contrib/pacsearch.in
> @@ -20,6 +20,7 @@
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
>  #TODO: colors flag on commandline
> +#TODO: for now the whole line is bold and some parts have the same 
> formatting (e.g. extra and version), maybe it would be better to use regular 
> formatting

No point changing this TODO and then removing it immediately in the next
patch.  Just remove it.

>  use strict;
>  use warnings;
> @@ -29,7 +30,7 @@ my $myver = '@PACKAGE_VERSION@';
>  
>  sub usage {
>       print "$myname (pacman) v$myver\n\n";
> -     print "Add color and install information to a 'pacman -Ss' search\n\n";
> +     print "Perform a pacman search using both the local and the sync 
> databases.\n\n";

Separate patch.  There are a few things in your three current patches
that should be done in this separate patch I will point out.

>       print "Usage: $myname <pattern>\n\n";
>       print "Example: $myname ^gnome\n";
>  }
> @@ -54,32 +55,38 @@ if ($ARGV[0] eq "--version" || $ARGV[0] eq "-V") {
>       exit 0;
>  }
>  
> -# define our colors to use when printing
> -my $CLR1 = "\e[0;34m";
> -my $CLR2 = "\e[0;32m";
> -my $CLR3 = "\e[0;35m";
> -my $CLR4 = "\e[0;36m";
> -my $CLR5 = "\e[0;31m";
> -my $CLR6 = "\e[0;33m";
> -my $CLR7 = "\e[1;36m";
> -my $INST = "\e[1;31m";
> -my $BASE = "\e[0m";
> -
> -# color a "repo/pkgname pkgver" line based on the repository name
> +# define formatting variables
> +my $FMT_BLUE = "\e[1;94m";
> +my $FMT_CYAN = "\e[1;96m";
> +my $FMT_GREEN = "\e[1;92m";
> +my $FMT_MAGENTA = "\e[1;95m";
> +my $FMT_RED = "\e[1;91m";
> +my $FMT_YELLOW = "\e[1;93m";
> +my $FMT_BOLD = "\e[1m";
> +my $FMT_RESET = "\e[0m";
> +

You name these FMT_BLUE etc and then rename them to BLUE in the next
patch.  Just use BLUE to be consistent with makepkg.

> +# Color a "repo/pkgname pkgver (goups) [installed]" line.

Typo: goups

> +# We try to stick to pacman colors.
>  sub to_color {
>       my $line = shift;
> -     # get the installed text colored first
> -     $line =~ s/(\[.*\]$)/$INST$1$BASE/;
> -     # and now the repo and dealings
> -     $line =~ s/(^core\/.*)/$CLR1$1$BASE/;
> -     $line =~ s/(^extra\/.*)/$CLR2$1$BASE/;
> -     $line =~ s/(^community\/.*)/$CLR3$1$BASE/;
> -     $line =~ s/(^testing\/.*)/$CLR4$1$BASE/;
> -     $line =~ s/(^community-testing\/.*)/$CLR5$1$BASE/;
> -     $line =~ s/(^multilib\/.*)/$CLR6$1$BASE/;
> -     $line =~ s/(^local\/.*)/$CLR7$1$BASE/;
> +     # get the installed text colored first (between square brackets)
> +     $line =~ s/(\[.*\]$)/$FMT_CYAN$1$FMT_RESET/;
> +     # group (between parentheses)
> +     $line =~ s/(\(.*\))/$FMT_BLUE$1$FMT_RESET/;

Do we need FTM_RESET every time?   Surely CYAN just overwrites BLUE.
More below...

> +     # version (second field)
> +     $line =~ s/^([^ ]+) ([^ ]+) /$1 $FMT_GREEN$2$FMT_RESET /;
> +     # name (word after slash)
> +     $line =~ s/\/([\w-]*)/\/$FMT_BOLD$1$FMT_RESET/;

In pacman's output, the whole line is bold.  Why are you not doing that
here?

> +     # repo (word before slash inclusive)
> +     $line =~ s/(^core\/)/$FMT_BLUE$1$FMT_RESET/;
> +     $line =~ s/(^extra\/)/$FMT_GREEN$1$FMT_RESET/;
> +     $line =~ s/(^community\/)/$FMT_MAGENTA$1$FMT_RESET/;
> +     $line =~ s/(^testing\/)/$FMT_CYAN$1$FMT_RESET/;
> +     $line =~ s/(^community-testing\/)/$FMT_RED$1$FMT_RESET/;
> +     $line =~ s/(^multilib\/)/$FMT_YELLOW$1$FMT_RESET/;
> +     $line =~ s/(^local\/)/$FMT_CYAN$1$FMT_RESET/;
>       # any other unknown repository
> -     $line =~ s/(^[\w-]*\/.*)/$CLR6$1$BASE/;
> +     $line =~ s/(^[\w-]*\/)/$FMT_YELLOW$1$FMT_RESET/;
>       return $line;
>  }
>  
> 


Reply via email to