implement feature "Support custom colours" #197

shv-q3 · 2017-08-18T16:08:09Z

used idea from googler project to add custom colors support
imported collections and used named tuple to store users colors from
"COLORMAP" which is identical with googler's project
separated vars in print_single_rec so color can be changed
separately for each item: id and title, url, desc, tag
add function to check for valid color strings and return error
msg if string is invalid
add check for os env defaults for colors that should be stored in $env:BUKU_COLORS
in args.colorstr ID_DB_str, URL_str, DESC_str, TAG_str
removes default colors and set users colors
updated man pages for colors option
updated auto-completions
updated README file under section "Usage"

# imported collections module for named tuple # add "colormap" from googler project # add named tuple for id and title, url, desc, tag # adjusted print_single_rec so colors can be changed for each item # add valid color string checker from googler # adjusted man options # add autocompletions

# sorted import in alphabetical order # adjusted var name's # add colors table to man file # add color os ENV option # add --color documentation in README under "Usage"

jarun · 2017-08-19T13:20:29Z

auto-completion/bash/buku-completion.bash

@@ -26,6 +26,7 @@ _buku () {
        -k --unlock
        -l --lock
        -m --merge
+        --colors


The options are in alphabetical order. Same for all the completion scripts.

as well as all completion scripts.

jarun · 2017-08-19T13:22:17Z

buku.1

@@ -218,6 +218,9 @@ Show selective monochrome output with specific fields. Works with --print. Searc
 .BI \-j " " \--json
 Output data formatted as json, works with --print output and search results.
 .TP
+.BI "--colors=" COLORS


Please use the following syntax like it's for other options in this man page:

.BI \--colors " COLORS"

jarun · 2017-08-19T13:44:17Z

buku.1

+.TE
+.TP
+.TP
+The default colors string is \fIGKlg\fR, which stands for


This looks awful. See my comment on line 3306 for more details.

jarun · 2017-08-19T13:45:34Z

buku.py

+        DESC_str = '%s   + %s\n'
+        TAG_str = '%s   # %s\n'
+        colors = Colors(*[COLORMAP[c] for c in args.colorstr], reset=COLORMAP['x'])
+        ID_DB_str = colors.ID_DB_str + ID_DB_str + COLORMAP['x']


The color for ID+title reflects in buku -p but not for the search results. Please fix.

I think you have not used ID_str. That's the problem.

Can we have something similar to googler?
ID - bold cyan (G)
Title - bold bright green (K)
DB Index (within []) - \x1b[2m (keep it hardcoded to dim)
URL - bright yellow (l)
comments/notes/desc - normal (x)
tags - blue (e)

Testing: Add a bookmark with url, title, tags and comment. Then

print it

search for it

to ensure your changes are good.

jarun · 2017-08-19T13:54:13Z

buku.py

@@ -3216,6 +3249,8 @@ def main():
    addarg('-p', '--print', nargs='*', help=HIDE)
    addarg('-f', '--format', type=int, default=0, choices={1, 2, 3, 4}, help=HIDE)
    addarg('-j', '--json', action='store_true', help=HIDE)
+    addarg('--colors', dest='colorstr', type=argparser.is_colorstr,
+           default=colorstr_env if colorstr_env else 'GKlg', metavar='COLORS', help=HIDE)


Please change. See my comment on line 3306 for more details.

jarun · 2017-08-19T13:58:07Z

buku.py

+        DESC_str = '%s   + %s\n'
+        TAG_str = '%s   # %s\n'
+        colors = Colors(*[COLORMAP[c] for c in args.colorstr], reset=COLORMAP['x'])
+        ID_DB_str = colors.ID_DB_str + ID_DB_str + COLORMAP['x']


I think you have not used ID_str. That's the problem.

jarun · 2017-08-19T14:17:26Z

buku.py

+        DESC_str = '%s   + %s\n'
+        TAG_str = '%s   # %s\n'
+        colors = Colors(*[COLORMAP[c] for c in args.colorstr], reset=COLORMAP['x'])
+        ID_DB_str = colors.ID_DB_str + ID_DB_str + COLORMAP['x']


Can we have something similar to googler?
ID - bold cyan (G)
Title - bold bright green (K)
DB Index (within []) - \x1b[2m (keep it hardcoded to dim)
URL - bright yellow (l)
comments/notes/desc - normal (x)
tags - blue (e)

Testing: Add a bookmark with url, title, tags and comment. Then

print it

search for it

to ensure your changes are good.

jarun · 2017-08-19T14:20:25Z

Please go through my comments and make the changes accordingly. This is a VERY important feature. Anything odd in default colors can put off all the users and kill the utility.

jarun · 2017-08-19T14:22:44Z

@ALL I would love to keep the defaults similar to googler but am open to suggestions. I would like everyone to review this PR.

professorjamesmoriarty · 2017-08-19T14:27:11Z

README.md

@@ -206,6 +206,7 @@ POWER TOYS:
                           N=1: URL, N=2: URL and tag, N=3: title,
                           N=4: URL, title and tag
      -j, --json           Json formatted output for -p and search
+      --colors             set output colors (see man page for details)


I'd make mention of this feature under the Features section as well.

It has to go in 2 places under features: README and man page.

Additions to the examples in both the README and man page may be helpful as well.

Please remove (see man page for details).

professorjamesmoriarty

Aside from my comment with regards to Features section addition. I love this.

mosegontar · 2017-08-20T21:35:28Z

buku.1

@@ -336,6 +339,75 @@ http[s]://[username:password@]proxyhost:proxyport/
 can be integrated in a GUI environment with simple tweaks. Refer to:
 .br
 .I https://github.com/jarun/Buku#gui-integration
+.SH COLORS
+\buku\fR allows you to customize the color scheme via a four-letter string, reminiscent of BSD \fBLSCOLORS\fR. The four letters represent the colors of


\buku\fR should be \fBBuku\fR, I think. It currently displays as k.

mosegontar · 2017-08-20T22:23:35Z

buku.py

+        DESC_str = colors.DESC_str + DESC_str + COLORMAP['x']
+        TAG_str = colors.TAG_str + TAG_str + COLORMAP['x']
+    else:
+        print('no')


Perhaps this should be a more explicit message? I'm not sure of the case that would trigger this, though. It seems like it might not be needed, since args.colorstr has a default value.

at least minimal debug msg

mosegontar · 2017-08-20T23:31:00Z

buku.py

+            for c in arg:
+                assert c in COLORMAP
+        except AssertionError:
+            raise argparse.ArgumentTypeError('%s is not a valid color string' % arg)


If an invalid color string is set as the env variable (e.g., ixejJ), the raised error message is buku.py: error: argument --colors: ixejJ is not a valid color string. Maybe not a problem, but this might be confusing since --colors is not used in that case by the user.

Please change it to ixejJ is not a valid color string.

mosegontar

I like this feature a lot. I made some suggestions, but the only imperative one on my end would be the typo in the man page.

rachmadaniHaryono · 2017-08-21T02:24:46Z

simple test would be helpful for is_colorstr would be helpful for maintenance.

rachmadaniHaryono

need test on is_colorstr

--color reordered in alphabetical order

jarun · 2017-08-22T02:06:13Z

auto-completion/fish/buku.fish

@@ -6,6 +6,7 @@
 #
 complete -c buku -s a -l add     -r --description 'add bookmark'
 complete -c buku -l ai              --description 'auto-import bookmarks'
+complete -c buku -l colors          --description 'set output colors (see man page for details)'


This takes an arg. Add -r.

Please remove (see man page for details).

jarun · 2017-08-22T02:07:20Z

auto-completion/zsh/_buku

@@ -11,6 +11,7 @@ local -a args
 args=(
    '(-a --add)'{-a,--add}'[add bookmark]:URL tags'
    '(--ai)--ai[auto-import bookmarks]'
+    '(--colors)--colors[set output colors (see man page for details)]'


Please change to:

'(--colors)--colors[set output colors]:color string'

Please remove (see man page for details).

jarun · 2017-08-22T02:08:47Z

auto-completion/bash/buku-completion.bash

@@ -13,6 +13,7 @@ _buku () {
    opts=(
        -a --add
        --ai
+        --colors


This should also go into the opts_with_arg group below.

add to opts_with_arg

jarun · 2017-08-22T07:16:18Z

README.md

@@ -206,6 +206,7 @@ POWER TOYS:
                           N=1: URL, N=2: URL and tag, N=3: title,
                           N=4: URL, title and tag
      -j, --json           Json formatted output for -p and search
+      --colors             set output colors (see man page for details)


Please remove (see man page for details).

jarun · 2017-08-22T07:16:24Z

auto-completion/fish/buku.fish

@@ -6,6 +6,7 @@
 #
 complete -c buku -s a -l add     -r --description 'add bookmark'
 complete -c buku -l ai              --description 'auto-import bookmarks'
+complete -c buku -l colors          --description 'set output colors (see man page for details)'


Please remove (see man page for details).

jarun · 2017-08-22T07:18:45Z

auto-completion/zsh/_buku

@@ -11,6 +11,7 @@ local -a args
 args=(
    '(-a --add)'{-a,--add}'[add bookmark]:URL tags'
    '(--ai)--ai[auto-import bookmarks]'
+    '(--colors)--colors[set output colors (see man page for details)]'


Please remove (see man page for details).

jarun · 2017-08-22T07:18:59Z

buku.py

@@ -3193,6 +3225,7 @@ def main():
                         N=1: URL, N=2: URL and tag, N=3: title,
                         N=4: URL, title and tag
    -j, --json           Json formatted output for -p and search
+    --colors             set output colors (see man page for details)


Please remove (see man page for details).

# separated id from ID_str and ID_DB_STR so it can have separate color # add 5 string color for (id, title, url, desc, tag) # adjusted print_single_rec for 5 colors result # fixed syntax .BI \--colors " COLORS" in man page # replaced four with five letter string for color # fixed default colors to "GKlxe" and "\x1b[2m" for DB index # add example to README and man page # removed check for colorstr_env set by user for the moment

jarun · 2017-08-22T18:57:42Z

README.md

@@ -192,6 +193,7 @@ ENCRYPTION OPTIONS:

 POWER TOYS:
      --ai                 auto-import from Firefox and Chrome
+      --colors             set output colors


For help, this was in the right position (just above --nc). My comment was only for the auto-complete scripts.

Also, please change it to:

--colors COLORS set output colors

misunderstanding no problem

Sure! Please take your time. This is a big PR, so nothing to hurry about.

jarun · 2017-08-22T19:01:15Z

buku.py

@@ -3211,6 +3222,7 @@ def main():
    power_grp = argparser.add_argument_group(
        title='POWER TOYS',
        description='''    --ai                 auto-import from Firefox and Chrome
+    --colors             set output colors


--colors COLORS set output colors

and please move it back to just before --nc

jarun · 2017-08-22T19:04:44Z

buku.1

@@ -340,19 +341,21 @@ can be integrated in a GUI environment with simple tweaks. Refer to:
 .br
 .I https://github.com/jarun/Buku#gui-integration
 .SH COLORS
-\buku\fR allows you to customize the color scheme via a four-letter string, reminiscent of BSD \fBLSCOLORS\fR. The four letters represent the colors of
+\fBBuku\fR allows you to customize the color scheme via a five-letter string, reminiscent of BSD \fBLSCOLORS\fR. The five letters represent the colors of


Please change the name to buku i.e., starting with lowercase char.

jarun · 2017-08-22T19:05:54Z

auto-completion/fish/buku.fish

@@ -6,7 +6,7 @@
 #
 complete -c buku -s a -l add     -r --description 'add bookmark'
 complete -c buku -l ai              --description 'auto-import bookmarks'
-complete -c buku -l colors          --description 'set output colors (see man page for details)'
+complete -c buku -l colors          --description 'set output colors'


Please change to:

complete -c buku -l colors -r --description 'set output colors'

The -r stands for required.

jarun · 2017-08-22T19:08:17Z

README.md

@@ -66,6 +66,7 @@ PRs are welcome. Please visit [#174](https://github.com/jarun/Buku/issues/174) f
 - Portable, merge-able database to sync between systems
 - Multithreaded full DB refresh
 - Shell completion scripts, man page with handy examples
+- Colored output


Please move it to the top point:

Lightweight, clean interface, custom colors

And add this in the man page too (we show the features in man page as well)

jarun · 2017-08-22T19:13:29Z

Also, flake8 check in Travis is failing due to some very long line. Please fix.

jarun · 2017-08-22T19:34:16Z

buku.1

@@ -182,6 +183,9 @@ Decrypt (unlock) the DB file with
 .BI \--ai
 Auto-import bookmarks from Firefox and Google Chrome.
 .TP
+.BI \--colors " COLORS"


Please move this to above --nc

shv-q3 · 2017-08-22T19:35:09Z

there is only 1 issue I found with the colors, when u search with -t option and don't put any args it displays the amount of bookmarks without color

jarun · 2017-08-22T19:39:03Z

buku.py

@@ -48,12 +49,27 @@
 colorize = True  # Allow color output by default

 # Default colour to print records
-ID_str = '\x1b[96;1m%d. \x1b[1;92m%s\x1b[0;2m [%s]\x1b[0m\n'
-ID_DB_str = '\x1b[96;1m%d. \x1b[1;92m%s\x1b[0m'
+ID_srch = '\x1b[36m%d.\x1b[0m'


The space after . is missing.

jarun · 2017-08-22T19:42:42Z

buku.py

-ID_str = '\x1b[96;1m%d. \x1b[1;92m%s\x1b[0;2m [%s]\x1b[0m\n'
-ID_DB_str = '\x1b[96;1m%d. \x1b[1;92m%s\x1b[0m'
+ID_srch = '\x1b[36m%d.\x1b[0m'
+ID_str = ID_srch + '\x1b[1;92m%s\x1b[2m [%s]\x1b[0m\n'


The DB ID is not showing in dim. In

2.GitHub - mewrev/dissection: The dissection of a simple "hello world" ELF binary. [305]

The [305] at the end is the DB id. It is shown in search results. You have to use \x1b[0;2m

jarun · 2017-08-22T20:14:43Z

README.md

@@ -66,7 +66,7 @@ PRs are welcome. Please visit [#174](https://github.com/jarun/Buku/issues/174) f
 - Portable, merge-able database to sync between systems
 - Multithreaded full DB refresh
 - Shell completion scripts, man page with handy examples
- Colored output
+- Lightweight, clean interface, custom colors


Lightweight, clean interface is already at the top of the feature list. Please append custom colors to it instead of a new entry at the end.

ah :) did not sleep yes i guess ok np

corrected in both files

jarun · 2017-08-22T20:16:22Z

README.md

@@ -208,6 +207,7 @@ POWER TOYS:
                           N=1: URL, N=2: URL and tag, N=3: title,
                           N=4: URL, title and tag
      -j, --json           Json formatted output for -p and search
+      --colors             --colors COLORS set output colors


Please check googler's help on --colors and copy it verbatim.

well googler's is:
--colors COLORS set output colors (see man page for details)
do you what to use this one?

OK. In that case just use whatever is there in googler.

jarun · 2017-08-22T20:18:05Z

buku.1

@@ -23,7 +23,7 @@ is a command-line utility to store, tag, search and organize bookmarks.
  * Portable, merge-able database to sync between systems
  * Multithreaded full DB refresh
  * Shell completion scripts, man page with handy examples
-  * Colored output
+  * Lightweight, clean interface, custom colors


Should be the topmost line.

jarun · 2017-08-22T20:19:29Z

buku.py

@@ -3237,6 +3236,7 @@ def main():
                         N=1: URL, N=2: URL and tag, N=3: title,
                         N=4: URL, title and tag
    -j, --json           Json formatted output for -p and search
+    --colors             --colors COLORS set output colors


jarun · 2017-08-22T22:09:59Z

I'll take care of the rest of the changes. Thanks!

shv-q3 added 4 commits August 18, 2017 17:43

Hotfix - implement "Support custom colours" from (jarun#174)

c4250e9

# sorted import in alphabetical order # adjusted var name's # add colors table to man file # add color os ENV option # add --color documentation in README under "Usage"

Merge remote-tracking branch 'remotes/upstream/master'

cb9a695

fix typo error in man page color section

1a893db

jarun requested changes Aug 19, 2017

View reviewed changes

jarun requested review from professorjamesmoriarty, rachmadaniHaryono and mosegontar August 19, 2017 14:21

professorjamesmoriarty reviewed Aug 19, 2017

View reviewed changes

professorjamesmoriarty approved these changes Aug 19, 2017

View reviewed changes

mosegontar reviewed Aug 20, 2017

View reviewed changes

mosegontar suggested changes Aug 20, 2017

View reviewed changes

rachmadaniHaryono requested changes Aug 21, 2017

View reviewed changes

shv-q3 added 4 commits August 22, 2017 01:36

Update buku-completion.bash

04404de

--color reordered in alphabetical order

Update buku.fish

9aea57a

--color reordered in alphabetical order

Update _buku

141660c

--color reordered in alphabetical order

Merge remote-tracking branch 'remotes/upstream/master'

5c07cc0

jarun requested changes Aug 22, 2017

View reviewed changes

shv-q3 added 2 commits August 22, 2017 20:34

Merge remote-tracking branch 'remotes/origin/master'

ff40af2

jarun requested changes Aug 22, 2017

View reviewed changes

jarun reviewed Aug 22, 2017

View reviewed changes

Fixed reviews

bf4e30d

jarun requested changes Aug 22, 2017

View reviewed changes

Review fixes

f263779

jarun merged commit 9dcbd67 into jarun:master Aug 22, 2017

github-actions bot locked and limited conversation to collaborators Jun 17, 2020

@@ @@ -13,6 +13,7 @@ _buku () { @@
                   opts=(
                       -a --add
                       --ai
+                      --colors

implement feature "Support custom colours" #197

implement feature "Support custom colours" #197

Conversation

shv-q3 commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun Aug 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun Aug 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun commented Aug 19, 2017 • edited Loading

jarun commented Aug 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

professorjamesmoriarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mosegontar Aug 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mosegontar left a comment

Choose a reason for hiding this comment

rachmadaniHaryono commented Aug 21, 2017

rachmadaniHaryono left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

jarun Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun commented Aug 22, 2017

Choose a reason for hiding this comment

shv-q3 commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun commented Aug 22, 2017

jarun Aug 19, 2017 •

edited

Loading

jarun Aug 19, 2017 •

edited

Loading

jarun commented Aug 19, 2017 •

edited

Loading

jarun Aug 22, 2017 •

edited

Loading

mosegontar Aug 20, 2017 •

edited

Loading

jarun Aug 22, 2017 •

edited

Loading

jarun Aug 22, 2017 •

edited

Loading

jarun Aug 22, 2017 •

edited

Loading