zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] Completion: Add _log_priorities, _logger
@ 2019-11-02  2:14 dana
  2019-11-02  5:54 ` Daniel Shahaf
  0 siblings, 1 reply; 6+ messages in thread
From: dana @ 2019-11-02  2:14 UTC (permalink / raw)
  To: Zsh hackers list

This patch does the following:

* Adds a new type function called _log_priorities, which completes
  syslog-style facilities and severity levels

* Updates the four functions i found that already take syslog priorities to
  use _log_priorities

* Adds a new function for the logger utility (which also uses _log_priorities
  obv)

I tried to make _log_priorities featureful enough to suit all of the obvious
use cases i could think of; let me know if it seems weird though. (The strange
option letters were, as with _bind_addresses, chosen to avoid conflicting with
compadd. Daniel once suggested that we come up with some standard calling
convention that resolves this head ache, but i haven't personally given it
much further thought)

This is also another case where i needed a host-with-port utility function
that properly handles IPv6 bracket syntax. I mentioned that i was working on
that a while ago, but i had trouble coming up with a good generalised API.
Might revisit some day

dana


diff --git a/Completion/Unix/Type/_log_priorities b/Completion/Unix/Type/_log_priorities
new file mode 100644
index 000000000..42cf485dc
--- /dev/null
+++ b/Completion/Unix/Type/_log_priorities
@@ -0,0 +1,70 @@
+#autoload
+
+# Complete syslog-style priorities/facilities/levels
+#
+# Note: 'Priority' according to syslog(3) refers to a severity level optionally
+# ORed with a facility. We use it here in a somewhat similar manner, as this
+# seems to cover the most ground, though we also support completing facilities
+# alone.
+#
+# By default, a case-insensitive facility.level pair is completed.
+#
+# Some tools that accept symbolic facility/level names also accept numbers
+# corresponding to their associated values defined in syslog.h. Since these
+# numbers vary across systems, we don't attempt to complete them.
+#
+# Options:
+#   -y  Complete only facilities
+#   -Y  Complete only severity levels
+#   -z  Complete only lower-case names
+#   -Z  Complete only upper-case names
+
+local -a expl facilities levels ca_opts
+local -A opts
+
+# Facilities vary across systems too, but these seem common enough
+facilities=(
+  kern
+  user
+  mail
+  daemon
+  auth
+  syslog
+  lpr
+  news
+  uucp
+  cron
+  authpriv
+  ftp
+  ntp
+  security
+  local{0..7}
+)
+levels=(
+  emerg   # 'panic' is deprecated
+  alert
+  crit
+  err     # 'error' is deprecated
+  warning # 'warn' is deprecated
+  notice
+  info
+  debug
+)
+
+zparseopts -A opts -D -E -- y Y z Z
+
+if (( $+opts[-Z] )); then
+  facilities=( ${(@U)facilities} )
+  levels=( ${(@U)levels} )
+elif (( ! $+opts[-z] )); then
+  ca_opts+=( -M 'm:{a-zA-Z}={A-Za-z}' )
+fi
+
+if (( $+opts[-Y] )) || { (( ! $+opts[-y] )) && compset -P '[^.]##.' }; then
+  _wanted levels expl 'log severity level' \
+      compadd -a "$@" "${(@)ca_opts}" - levels
+else
+  (( ! $+opts[-y] )) && ! compset -S '.[^.]##' && ca_opts+=( -qS. )
+  _wanted facilities expl 'log facility' \
+      compadd -a "$@" "${(@)ca_opts}" - facilities
+fi

diff --git a/Completion/BSD/Command/_pfctl b/Completion/BSD/Command/_pfctl
index 23898882f..a2a81b5db 100644
--- a/Completion/BSD/Command/_pfctl
+++ b/Completion/BSD/Command/_pfctl
@@ -58,9 +58,7 @@ _pf_tables() {
 
 case $OSTYPE in
   openbsd*)
-    pfctl_debug_level=(
-      emerg alert crit err warning notice info debug
-    )
+    _log_priorities -Y -O pfctl_debug_level
     args=(
       '-L+[load pf states from specified state file]:file:_files'
       "-N[don't perform domain name resolution]"

diff --git a/Completion/Linux/Command/_iptables b/Completion/Linux/Command/_iptables
index 27c801da1..d4d678579 100644
--- a/Completion/Linux/Command/_iptables
+++ b/Completion/Linux/Command/_iptables
@@ -57,7 +57,7 @@ case ${prev[${prev[(I)-j|--jump]}+1]}; in
   ECN) args+=( '--ecn-tcp-remove[remove all ECN bits from TCP header]' ) ;;
   LOG)
     args+=(
-      '--log-level[specify level of logging]:log level:(debug info notice warning err crit alert emerg)'
+      '--log-level[specify level of logging]: :_log_priorities -Y'
       '--log-prefix[specify prefix string for log message]:string'
       '--log-tcp-sequence[log TCP sequence numbers]'
       '--log-tcp-options[log TCP options]'

diff --git a/Completion/Unix/Command/_rclone b/Completion/Unix/Command/_rclone
index 01d851fa1..698a94772 100644
--- a/Completion/Unix/Command/_rclone
+++ b/Completion/Unix/Command/_rclone
@@ -147,7 +147,7 @@ _arguments -C \
   '--streaming-upload-cutoff[specify size cutoff for switching to chunked upload]:size [100k]' \
   '--suffix[specify suffix for use with --backup-dir]:string' \
   '--syslog[use syslog for logging]' \
-  '--syslog-facility[facility for syslog, eg KERN,USER,...]:string [DAEMON]' \
+  '--syslog-facility[specify syslog facility]:log facility [DAEMON]:_log_priorities -yZ' \
   '--timeout[specify IO idle timeout]:duration [5m0s]' \
   '--tpslimit[limit HTTP transactions per second to this]:float' \
   '--tpslimit-burst[max burst of transactions for --tpslimit]:int [1]' \

diff --git a/Completion/Unix/Command/_ssh b/Completion/Unix/Command/_ssh
index 0775590e6..cc7ed563c 100644
--- a/Completion/Unix/Command/_ssh
+++ b/Completion/Unix/Command/_ssh
@@ -441,7 +441,7 @@ _ssh () {
           _wanted values expl 'value' compadd yes no ask accept-new off && ret=0
           ;;
         (#i)syslogfacility=*)
-          _wanted facilities expl 'facility' compadd -M 'm:{a-z}={A-Z}' DAEMON USER AUTH LOCAL{0,1,2,3,4,5,6,7} && ret=0
+          _log_priorities -y && ret=0
           ;;
         (#i)(verifyhostkeydns|updatehostkeys)=*)
           _wanted values expl 'truthish value' compadd yes no ask && ret=0

diff --git a/Completion/Unix/Command/_logger b/Completion/Unix/Command/_logger
new file mode 100644
index 000000000..180e35b89
--- /dev/null
+++ b/Completion/Unix/Command/_logger
@@ -0,0 +1,93 @@
+#compdef logger
+
+local variant
+local -a args aopts=( -A '-*' )
+
+_pick_variant -r variant util-linux=util-linux $OSTYPE --version
+
+case $variant in
+  *)
+    args+=(
+      '(-f -e --file --journald --prio-prefix --skip-empty)*: :_guard "^-*" message'
+      '(: * --journald)'{-f+,--file=}'[log contents of specified file]: :_files'
+      '(--id)-i[log with PID of logger process]'
+      '(-p --priority)'{-p+,--priority=}'[log with specified priority]: :_log_priorities'
+      '(-t --tag)'{-t+,--tag=}'[log with specified tag]:tag'
+    )
+    ;|
+  darwin*|dragonfly*|freebsd*|linux*|netbsd*|openbsd*)
+    args+=(
+      '(-s --stderr)'{-s,--stderr}'[also log to stderr (LOG_PERROR)]'
+    )
+    ;|
+  netbsd*|openbsd*)
+    args+=(
+      '-c[log to console if unable to log normally (LOG_CONS)]'
+    )
+    ;|
+  dragonfly*|freebsd*)
+    args+=(
+      '(-6)-4[log via IPv4 only]'
+      '(-4)-6[log via IPv6 only]'
+      '-A[log to all IP addresses associated with remote host name]'
+      # @todo Complete UNIX-domain sockets
+      '-h+[log to specified remote host or UNIX-domain socket]:remote syslog host:_hosts'
+    )
+    ;|
+  freebsd*)
+    args+=(
+      '-H+[specify host name for message header]: :_hosts'
+      '-P+[log to specified port (with -h)]:remote syslog port:_ports'
+      # @todo This is another case where we need a dedicated _hosts_ports
+      # function which can properly handle bracketed IPv6 address syntax
+      '-S+[specify source address/port (with -h)]: :_bind_addresses -0bh'
+    )
+    ;;
+  netbsd*)
+    args+=(
+      '-d+[log specified structured data]:structured data'
+      '-m+[specify RFC5424 MSGID]:MSGID'
+      '-n[open log connection immediately (LOG_NDELAY)]'
+    )
+    ;;
+  util-linux)
+    aopts=( )
+    args+=(
+      '(-d -T --tcp --udp)'{-d,--udp}'[log via UDP only (with -n)]'
+      '(: * -e --skip-empty)'{-e,--skip-empty}'[ignore empty lines in input (with -f or stdin)]'
+      '(-i)--id=-[log with PID of logger process or specified PID]:: :_pids'
+      '(--rfc3164)--msgid=[specify RFC5424 MSGID]:MSGID'
+      '(-n --server)'{-n+,--server=}'[log to specified remote host]:remote syslog host:_hosts'
+      '--no-act[do not actually log]'
+      '--octet-count[use RFC6587 octet counting]'
+      '(-P --port)'{-P+,--port=}'[log to specified port (with -n)]:remote syslog port:_ports'
+      '(: *)--prio-prefix[look for priority prefix <n> on each line (with -f or stdin)]'
+      '(--msgid --rfc5424 --sd-id --sd-param)--rfc3164[use RFC3164 BSD syslog protocol]'
+      '(--rfc3164)--rfc5424=-[use RFC5424 syslog protocol with optional exceptions]:protocol exceptions:((
+        nohost\:"suppress gethostname(2) information"
+        notime\:"suppress complete sender time stamp (implies notq)"
+        notq\:"suppress time-quality structured data"
+      ))'
+      # @todo We could do a better job of completing these next two. The
+      # util-linux man page describes the format and some of the known values.
+      # Also, this is one of those cases where the options have to be given in
+      # order, i.e., each --sd-param must be preceded by --sd-id and vice versa
+      '(--rfc3164)*--sd-id=[specify RFC5424 structured-data element ID]:structured-data element ID'
+      '(--rfc3164)*--sd-param=[specify RFC5424 structured-data element parameter]:structured-data element parameter (name=value)'
+      '--size=[specify maximum message size]:message size (bytes)'
+      '--socket-errors=-[specify socket error-printing behavior]:mode::(auto off on)'
+      '(-u --socket)'{-u+,--socket=}'[write to specified socket instead of default]:syslog socket:_files -g "*(-=D)"'
+      '(-d -T --tcp --udp)'{-t,--tcp}'[log via TCP only (with -n)]'
+      '(: * -)'{-V,--version}'[display version information]'
+      '(: * -)'{-h,--help}'[display help information]'
+    )
+    # This option actually ignores many other options, like -p
+    (( $+commands[journalctl] )) && args+=(
+      '(: * -f --file)--journald=-[log systemd journal entry from stdin or specified file]: ::_files'
+    )
+    ;;
+esac
+
+[[ $variant == util-linux ]] || args=( ${args:#(\([^\)]#\)|)(\*|)--*} )
+
+_arguments -s -S $aopts : $args


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Completion: Add _log_priorities, _logger
  2019-11-02  2:14 [PATCH] Completion: Add _log_priorities, _logger dana
@ 2019-11-02  5:54 ` Daniel Shahaf
  2019-11-02  6:45   ` dana
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Shahaf @ 2019-11-02  5:54 UTC (permalink / raw)
  To: dana; +Cc: Zsh hackers list

dana wrote on Fri, Nov 01, 2019 at 21:14:46 -0500:
> (The strange option letters were, as with _bind_addresses, chosen to avoid
> conflicting with compadd. Daniel once suggested that we come up with some
> standard calling convention that resolves this head ache, but i haven't
> personally given it much further thought)

The first idea that comes to mind here is to use one of these conventions:
.
    _log_priorities "${compadd_options[@]}" -Y --foo=lorem -Y --foo -Y lorem
.
or
.
    _log_priorities "${compadd_options[@]}" -Y '--foo=lorem --foo lorem'

That is: use up _one_ top-level option letter and stuff all the not-compadd
--options and their arguments into the argument(s) of that top-level option
letter.

We can also do:
.
    _log_priorities "${compadd_options[@]}" -- "${compadd_positional_arguments[@]}" -- "${log_priorities_argv[@]}"
.
which is similar to what zargs does.  Normally there would be a question here
of how to escape a literal "--" in $compadd_positional_arguments, but for
compadd specifically, I think the following is a satisfactory workaround:
.
    _log_priorities -a compadd_positional_arguments "${compadd_options[@]}" -- -- "${log_priorities_argv[@]}"

I'll leave it to you to decide whether it's worthwhile to change
_log_priorities from the version you posted.

Which reminds me: Shouldn't we mention _log_priorities in the manual, it being
a new Type/ function?

Cheers,

Daniel

> --- /dev/null
> +++ b/Completion/Unix/Type/_log_priorities
> @@ -0,0 +1,70 @@
> +#autoload
> +
> +# Complete syslog-style priorities/facilities/levels
> +#
> +# Note: 'Priority' according to syslog(3) refers to a severity level optionally
> +# ORed with a facility. We use it here in a somewhat similar manner, as this
> +# seems to cover the most ground, though we also support completing facilities
> +# alone.
> +#
> +# By default, a case-insensitive facility.level pair is completed.
> +#
> +# Some tools that accept symbolic facility/level names also accept numbers
> +# corresponding to their associated values defined in syslog.h. Since these
> +# numbers vary across systems, we don't attempt to complete them.
> +#
> +# Options:
> +#   -y  Complete only facilities
> +#   -Y  Complete only severity levels
> +#   -z  Complete only lower-case names
> +#   -Z  Complete only upper-case names

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Completion: Add _log_priorities, _logger
  2019-11-02  5:54 ` Daniel Shahaf
@ 2019-11-02  6:45   ` dana
  2019-11-02  6:58     ` dana
  0 siblings, 1 reply; 6+ messages in thread
From: dana @ 2019-11-02  6:45 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

On 2 Nov 2019, at 00:54, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> That is: use up _one_ top-level option letter and stuff all the not-compadd
> --options and their arguments into the argument(s) of that top-level option
> letter.

That would definitely work, and be easy to parse, though it is a bit
unattractive, especially given how these functions are often called. e.g.,
you could end up with _arguments specs like:

  '--foo=[specify foo]: :_my_type -Y "-a \"my optarg\" -bcd"'

Granted, very few type functions have options that take arguments, so it
probably wouldn't come up that often, but...

On 2 Nov 2019, at 00:54, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> We can also do:
> .
>    _log_priorities "${compadd_options[@]}" -- "${compadd_positional_arguments[@]}" -- "${log_priorities_argv[@]}"
> .
> which is similar to what zargs does.  Normally there would be a question here
> of how to escape a literal "--" in $compadd_positional_arguments, but for
> compadd specifically, I think the following is a satisfactory workaround:

Aside from the literal -- operand thing, there are a few complicating factors
i can think of with that:

1. Lower-level functions like _arguments want to automatically stick compadd
   options at the beginning of those functions' argument lists, so either they
   would have to be changed to pass the two --s (potentially causing issues
   for existing type functions that don't expect them) or you would have to
   include the --s in every argument spec you wrote.

2. Related to what you mentioned: Because compadd options go at the beginning,
   whatever is parsing the function's arguments needs to know all about what
   options compadd takes; otherwise it could get confused by an optarg of --.

3. Your type function has to figure out how to parse all of this, which could
   lead to a lot of noisy and possibly error-prone boiler-plate.

I think #2 and #3 could be solved by a helper function that does like a
two-pass zparseopts, but i'm not sure about #1, or about the path forward for
existing functions in general. (Though, one benefit of your first suggestion,
if we made sure to pick a letter that no type functions currently want for
themselves, is that it would be pretty straight-forward to just update them as
we go, and then maybe some years in the future we could deprecate/remove the
old way of doing it...?)

On 2 Nov 2019, at 00:54, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> I'll leave it to you to decide whether it's worthwhile to change
> _log_priorities from the version you posted.

I'm definitely open to it, just want to make sure it's something everybody
agrees with that'll work going forward.

On 2 Nov 2019, at 00:54, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Which reminds me: Shouldn't we mention _log_priorities in the manual, it
> being a new Type/ function?

I'm not actually sure. zshcompsys mentions a few type functions, but they're
only the really general ones like _files. Aside from some passing mentions,
there's nothing in the manual for stuff like _hosts and _pids and _signals. I
always assumed that was a deliberate omission, so i've never bothered with any
of the new type functions i've added.

dana


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Completion: Add _log_priorities, _logger
  2019-11-02  6:45   ` dana
@ 2019-11-02  6:58     ` dana
  2019-11-02  7:45       ` Daniel Shahaf
  0 siblings, 1 reply; 6+ messages in thread
From: dana @ 2019-11-02  6:58 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

On 2 Nov 2019, at 01:45, dana <dana@dana.is> wrote:
> That would definitely work, and be easy to parse, though it is a bit
> unattractive, especially given how these functions are often called.

Oh, i realised with your *very* first suggestion, the quoting wouldn't be
quite as bad:

  '--foo=[specify foo]: :_my_type -Y-a -Y"my optarg" -Y-bcd'

Though it's certainly strange in a different way.

It would be cool if there was a convenient way to just have a general prefix
for options, so that this kind of thing would work:

  '--foo=[specify foo]: :_my_type -Ya "my optarg" -Ybcd'

(The point being that (a) you don't have to repeat the hyphens and (b) it
knows that "my optarg" is an argument to the type function's -a.)

Maybe zparseopts could be extended to do that somehow, if it's not too silly
of an idea.

dana


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Completion: Add _log_priorities, _logger
  2019-11-02  6:58     ` dana
@ 2019-11-02  7:45       ` Daniel Shahaf
  2019-11-02  8:20         ` dana
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Shahaf @ 2019-11-02  7:45 UTC (permalink / raw)
  To: zsh-workers

dana wrote on Sat, 02 Nov 2019 06:58 +00:00:
> On 2 Nov 2019, at 01:45, dana <dana@dana.is> wrote:
> > That would definitely work, and be easy to parse, though it is a bit
> > unattractive, especially given how these functions are often called.
> 
> Oh, i realised with your *very* first suggestion, the quoting wouldn't be
> quite as bad:
> 
>   '--foo=[specify foo]: :_my_type -Y-a -Y"my optarg" -Y-bcd'
> 
> Though it's certainly strange in a different way.

Strange how?  I think it's pretty intuitive: to call «_foo "$bar[@]"»,
write «_foo -Y"${^bar[@]}"» (?).  This is:

- Completely generic: it doesn't depend on the value of «"${bar[@]}"»,
  whether it uses single-letter options or GNU-style long options or
  anything else.

- Composable: it's trivial to pass -bcd through _two_ layers of these
  wrappers, should that ever be necessary.  (Example: pdebuild(1) calls
  dpkg-buildpackage(1) which in turn calls dpkg-genbuildinfo(1), and
  it's possible to pass dpkg-genbuildinfo(1) arguments to pdebuild(1).)

- Requires no new code to implement.  It's simply a repeatable option
  whose optargs are collected in an array.  Both zparseopts and _arguments
  already support this.

> It would be cool if there was a convenient way to just have a general prefix
> for options, so that this kind of thing would work:
> 
>   '--foo=[specify foo]: :_my_type -Ya "my optarg" -Ybcd'
> 
> (The point being that (a) you don't have to repeat the hyphens and (b) it
> knows that "my optarg" is an argument to the type function's -a.)
> 
> Maybe zparseopts could be extended to do that somehow, if it's not too silly
> of an idea.

*nod*

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Completion: Add _log_priorities, _logger
  2019-11-02  7:45       ` Daniel Shahaf
@ 2019-11-02  8:20         ` dana
  0 siblings, 0 replies; 6+ messages in thread
From: dana @ 2019-11-02  8:20 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2 Nov 2019, at 02:45, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Strange how?  I think it's pretty intuitive: to call «_foo "$bar[@]"»,
> write «_foo -Y"${^bar[@]}"» (?).  This is...
> - Completely generic...
> - Composable...
> - Requires no new code to implement

All true, yeah. And you kind of implied it in your first bullet, but, to call
attention to it, a nice thing about that method is that it also extends to
(positional) operands. The -Ya thing i suggested wouldn't do that. (On the
other hand you pretty much never want to pass operands through a type function
to compadd, so idk if it matters.)

My main (only?) issue with it is just that it looks a bit visually messy to
me. Calling it with -Y$^arr is succinct enough, but you'll rarely do that in
practice, as arguments to type functions are usually written out at the point
of call. So you'd see a lot more of the `-Y-a -Y"my optarg" -Y-bcd` kind
of stuff.

dana


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-02  8:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-02  2:14 [PATCH] Completion: Add _log_priorities, _logger dana
2019-11-02  5:54 ` Daniel Shahaf
2019-11-02  6:45   ` dana
2019-11-02  6:58     ` dana
2019-11-02  7:45       ` Daniel Shahaf
2019-11-02  8:20         ` dana

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).