Gentoo Forums
Gentoo Forums
Gentoo Forums
Quick Search: in
[Patch] Pager support for "eselect news read"
View unanswered posts
View posts from last 24 hours

 
Reply to topic    Gentoo Forums Forum Index Unsupported Software
View previous topic :: View next topic  
Author Message
teika
Tux's lil' helper
Tux's lil' helper


Joined: 19 Feb 2011
Posts: 90
Location: Teika rhymes w/ "Stay blur", not ψ̄

PostPosted: Sat Feb 04, 2017 1:49 am    Post subject: [Patch] Pager support for "eselect news read" Reply with quote

Now you can use a pager for "eselect news read". When PAGER is set, that value will be used. If you do $ eselect news read > some/file, then a pager won't be used.

You can get news.eselect, and put it at /usr/share/eselect/modules/, or get a patch, pager.patch. Do $ cd /usr/share/eselect/modules/; patch -p2 < pager.patch, and you're done. The patch is applicable to 1.4.5 <= app-admin/eselect <= 1.4.8, at least. (I recommend to use patches, because patches are easier to check if they're safe. This one has only 22 lines insertion.).

How to test:
$ eselect news read 1 2 3

I'll post a patch to the bugzilla, but I think I should hear comments before doing that. Please give me some feedback here. Even a couple of "seems ok" like replies are welcome, rather than nothing.

Regards.
# I've always been annoyed by the lack of the pager in "eselect news read". Why hasn't anyone tried to fix it?
_________________
Easy Shift / Ctrl / AltGr ... hack; save your pinkies, type without drudgery: topic 865313
Trouble upgrading Perl? See topic 1058924.
Back to top
View user's profile Send private message
alinefr
Tux's lil' helper
Tux's lil' helper


Joined: 05 Jul 2009
Posts: 102
Location: São Paulo, Brasil

PostPosted: Sun Feb 05, 2017 7:42 pm    Post subject: Reply with quote

Hi @teika, thanks for sharing this.

Is your gitlab repo public? I get 404 when I click either in "news.eselect" or "pager.patch".
Back to top
View user's profile Send private message
teika
Tux's lil' helper
Tux's lil' helper


Joined: 19 Feb 2011
Posts: 90
Location: Teika rhymes w/ "Stay blur", not ψ̄

PostPosted: Sun Feb 05, 2017 11:31 pm    Post subject: Reply with quote

O, desculpe. (=Excuse me) Yes, it was inaccessible (private), but now fixed. Obligado for telling.
Back to top
View user's profile Send private message
steveL
Advocate
Advocate


Joined: 13 Sep 2006
Posts: 4556
Location: The Peanut Gallery

PostPosted: Tue Feb 07, 2017 9:46 pm    Post subject: Reply with quote

Good idea, teika :-)

Your quoting is a bit suspect, so I'll go over a bit (hopefully that will be helpful to you.)
Code:
+   declare -a tmpfiles
+   local tmpDir=$(mktemp --tmpdir -d eselect.news.XXX)
+   local tmp
+
This is a bit messy; in general, lay out your locals without parameter expansion, and you don't need declare -a, but you would for an associative array (with -A.)
I'd do something like:
Code:
local tmp tmpdir tmpfiles=()
tmpdir=$(mktemp --tmpdir -d eselect.news.XXX) || die 'unable to create tmpdir'
I changed the naming style to match tmpfiles as it looks odd otherwise.
Code:
+      tmp="${tmpDir}/${i}"
+      tmpfiles=( ${tmpfiles[@]} $tmp )
+      exec 1>${tmp}
The second line here is dangerous, in the general case (both expansions need quoting), and there is a shortcut +=(..) operator in bash (again you must quote expansions for safety.)
The first line illustrates one of the only two places that POSIX sh specifies no word-splitting (while parameter expansion still applies): assignment -- consider assignment of a command substitution eg mktemp; it would be crufty to need quoting. The other is: case $foo in ..
The last does not require the 1 as the default fd for output redirection is 1 for stdout, but does require quoting.
Code:
      tmp=$tmpdir/$i
      tmpfiles+=("$tmp")
      exec >"$tmp"
Redundant braces only distract from the main focus of robust shell scripting: correct quoting, especially of parameter expansions.
Code:
+   exec 1>&3 3>&-
+
+   local do_read_pager=${PAGER:-cat}
+
+   if [[ -n "${tmpfiles[@]}" ]]; then
+      if [[ -t 1 ]]; then
+          $do_read_pager ${tmpfiles[@]}
+      else
+          cat ${tmpfiles[@]}
+      fi
+      rmdir -rf $tmpDir
+   fi
+
Bad quoting, which is not needed in the [[ construct (you'd want a [*] expansion, not [@]), which can be done quicker.
Test for interactive needs to test stdin as well, usual test is that and stderr; and let's not bother with isatty calls unless we need to, which simplifies things.
No quoting on tmpDir (no arguments about what it contains right at this point; it could easily break under prefix for example.)
Code:
   exec >&3 3>&-
   if ((${#tmpfiles[*]} )); then
      if [[ -n $PAGER ]] && [[ -t 0 && -t 2 ]]; then
          $PAGER "${tmpfiles[@]}"
      else
          cat "${tmpfiles[@]}"
      fi
   fi
   rm -fr "$tmpdir"
The rmdir needs to come out of the if, as you create it unconditionally (not sure what rmdir is, but rm -fr is portable; switch back if it's an internal function.)
Main thing though is quoting of arrays, to pass as separate parameters properly, and as ever: quote your parameter expansions unless you specifically intend (to allow) bash to word-split them, or as above (for bash/ksh) you are using [[ and it is not the right hand side of an = (or ==, though we don't use that.)

HTH.

edit: mistaken note of caution (see below)


Last edited by steveL on Fri Feb 10, 2017 5:52 am; edited 1 time in total
Back to top
View user's profile Send private message
teika
Tux's lil' helper
Tux's lil' helper


Joined: 19 Feb 2011
Posts: 90
Location: Teika rhymes w/ "Stay blur", not ψ̄

PostPosted: Thu Feb 09, 2017 1:42 am    Post subject: Reply with quote

Ok, the new version is now downloadable.

@steveL: I'm indeed grateful for a warm, valuable comment. Such pedagogy should really help not only me, but the entire community. (Well, it sounds somewhat formal, sorry, but I'm not sure of more polite expression than "thank you very much".)

:P Now everyone knows how I live with my bash. Me either dunno what's "rmdir -fr", neither why it worked.
Back to top
View user's profile Send private message
steveL
Advocate
Advocate


Joined: 13 Sep 2006
Posts: 4556
Location: The Peanut Gallery

PostPosted: Fri Feb 10, 2017 5:51 am    Post subject: Reply with quote

teika wrote:
I'm not sure of more polite expression than "thank you very much".

You're very welcome, teika; that kind of thanks is what motivates me to try and help others. :-)

I think rmdir is a GNU thing, matching the rmdir syscall; rm -fr is portable across POSIX.

Noticed a mistake here.
steveL wrote:
Note: don't use ((${#arr[*]} )) for an associative array; use (("${#arr[@]}" )) instead which is slower on integer-indexed arrays.

This is incorrect; both work (and the first is quicker, from previous testing.)
It was bugging me, so tried it in terminal with:
Code:
declare -A arr; arr['foo bar']='foo bar'; arr['baz quux']='baz quux'

I was thinking of
Code:
for i in "${!arr[@]}"; do sth -with "${arr["$i"]}"; done
which is how we step through associative arrays (note the quoting of ["$i"]), whereas:
Code:
for i in ${!arr[*]}; do sth -with "${arr[i]}"; done
works quickest for integer-indexed arrays (default).
Both step through the indexes of the array. For integer-indexed arrays these may be sparse; ie gaps between integral values. For associative arrays, there is no ordering, but the index is a string value and must be quoted as such.

If you know an integer-indexed array is not sparse, eg a local you've built up with arr+=("$foo") as here, or/from a defined: arr=('fixed set' "$foo") (readonly is useful for globals), you can use:
Code:
for ((n=${#arr[*]}, i = 0; i < n; i++)); do sth -with "${arr[i]}"; done

Replace "sth -with" with echo to see what I mean, in terminal. You will need to unset arr in-between.

The essential thing to bear in mind, is that an associative array index is a string, of any non-zero-byte data, so must be treated as carefully as we treat other parameters.

Also, if you weren't aware, the index part of an array access, for integer-indexed arrays, is actually an arithmetic context, same as inside ((..)) which is why we can use 'i' as is without any $ in front of it; for an integer-indexed array that is.
So we can do, for instance:
Code:
n=${#cmd[*]}; base=$n; cmd[n++]=''; .. cmd[n++]=$i; cmd[n++]=$foo
which comes in handy when building up dialog commands in stages, for one example. (The base index is a placeholder we fill in later, and the latter assignments happen in a loop.)
Back to top
View user's profile Send private message
Display posts from previous:   
Reply to topic    Gentoo Forums Forum Index Unsupported Software All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum