diff mbox series

[2/2] ipsec: also show only valid options in the autocompletion

Message ID 1503934373-8996-2-git-send-email-jonatan.schlag@ipfire.org
State New
Headers
Series [1/2] ipsec: only accept valid options for a connection type |

Commit Message

Jonatan Schlag Aug. 29, 2017, 1:32 a.m. UTC 
  The options for the different types of connections should be also printed
correctly when we use autocompletion

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 src/bash-completion/network | 17 ++++++++++++++++-
 src/network                 |  3 +++
 2 files changed, 19 insertions(+), 1 deletion(-)

Comments

Michael Tremer Sept. 8, 2017, 5:45 a.m. UTC | #1 
Hi,

technically this work, but is it not better to have a command that checks what
type a connection is (rw or n2n) and then the right list of commands will be
selected?

I think that is cleaner instead of heaving some auto-completion stuff in the
main library.

Best,
-Michael

On Mon, 2017-08-28 at 17:32 +0200, Jonatan Schlag wrote:
> The options for the different types of connections should be also printed
> correctly when we use autocompletion
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/bash-completion/network | 17 ++++++++++++++++-
>  src/network                 |  3 +++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/src/bash-completion/network b/src/bash-completion/network
> index 71bf245..25cf6aa 100644
> --- a/src/bash-completion/network
> +++ b/src/bash-completion/network
> @@ -413,7 +413,7 @@ _network_vpn_ipsec_connection_subcommands() {
>  	shift
>  	local words=( $@ )
>  
> -	local commands="authentication color description down inactivity-
> timeout local mode peer remote security-policy show up"
> +	local commands="$(network raw list-valid-ipsec-options
> "${connection}" "first")"
>  	local cmd="$(_network_find_on_cmdline "${commands}")"
>  	if [[ -z "${cmd}" ]]; then
>  		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
> @@ -432,6 +432,9 @@ _network_vpn_ipsec_connection_subcommands() {
>  		description)
>  			_network_description ${args}
>  			;;
> +		dpd)
> +			_network_vpn_ipsec_connection_subcommands_dpd
> "${connection}" ${args}
> +			;;
>  		local)
>  			_network_vpn_ipsec_connection_subcommands_local_remot
> e ${connection} "local" ${args}
>  			;;
> @@ -529,6 +532,18 @@
> _network_vpn_ipsec_connection_subcommands_security_policy() {
>  	fi
>  }
>  
> +_network_vpn_ipsec_connection_subcommands_dpd() {
> +	local connection=${1}
> +	shift
> +	local words=( $@ )
> +
> +	local commands="$(network raw list-valid-ipsec-options
> "${connection}" "dpd")"
> +	local cmd="$(_network_find_on_cmdline "${commands}")"
> +	if [[ -z "${cmd}" ]]; then
> +		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
> +		return 0
> +	fi
> +}
>  _network_vpn_security_policies() {
>  	local words=( $@ )
>  
> diff --git a/src/network b/src/network
> index 9a2d480..379a71d 100644
> --- a/src/network
> +++ b/src/network
> @@ -1334,6 +1334,9 @@ cli_raw() {
>  		list-next-free-zones)
>  			zones_get_next_free
>  			;;
> +		list-valid-ipsec-options)
> +			ipsec_get_valid_options "$@"
> +			;;
>  		list-zone-config-ids)
>  			zone_config_list_ids "$@"
>  			;;
Jonatan Schlag Sept. 8, 2017, 7:14 a.m. UTC | #2 
Hi

Am Do, 7. Sep, 2017 um 9:45 schrieb Michael Tremer 
<michael.tremer@ipfire.org>:
> Hi,
> 
> technically this work, but is it not better to have a command that 
> checks what
> type a connection is (rw or n2n) and then the right list of commands 
> will be
> selected?
> 
> I think that is cleaner instead of heaving some auto-completion stuff 
> in the
> main library.

IIt may be cleaner but so we only one place where have all possible 
options listed. This makes adding or changing options much smarter 
because we have only one place where all options are listed. So I think 
we should keep this solution and have an n extra variable in the 
auto-completion wish we have to update on every change.

Jonatan
> 
> Best,
> -Michael
> 
> On Mon, 2017-08-28 at 17:32 +0200, Jonatan Schlag wrote:
>>  The options for the different types of connections should be also 
>> printed
>>  correctly when we use autocompletion
>> 
>>  Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
>>  ---
>>   src/bash-completion/network | 17 ++++++++++++++++-
>>   src/network                 |  3 +++
>>   2 files changed, 19 insertions(+), 1 deletion(-)
>> 
>>  diff --git a/src/bash-completion/network 
>> b/src/bash-completion/network
>>  index 71bf245..25cf6aa 100644
>>  --- a/src/bash-completion/network
>>  +++ b/src/bash-completion/network
>>  @@ -413,7 +413,7 @@ _network_vpn_ipsec_connection_subcommands() {
>>   	shift
>>   	local words=( $@ )
>> 
>>  -	local commands="authentication color description down inactivity-
>>  timeout local mode peer remote security-policy show up"
>>  +	local commands="$(network raw list-valid-ipsec-options
>>  "${connection}" "first")"
>>   	local cmd="$(_network_find_on_cmdline "${commands}")"
>>   	if [[ -z "${cmd}" ]]; then
>>   		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
>>  @@ -432,6 +432,9 @@ _network_vpn_ipsec_connection_subcommands() {
>>   		description)
>>   			_network_description ${args}
>>   			;;
>>  +		dpd)
>>  +			_network_vpn_ipsec_connection_subcommands_dpd
>>  "${connection}" ${args}
>>  +			;;
>>   		local)
>>   			_network_vpn_ipsec_connection_subcommands_local_remot
>>  e ${connection} "local" ${args}
>>   			;;
>>  @@ -529,6 +532,18 @@
>>  _network_vpn_ipsec_connection_subcommands_security_policy() {
>>   	fi
>>   }
>> 
>>  +_network_vpn_ipsec_connection_subcommands_dpd() {
>>  +	local connection=${1}
>>  +	shift
>>  +	local words=( $@ )
>>  +
>>  +	local commands="$(network raw list-valid-ipsec-options
>>  "${connection}" "dpd")"
>>  +	local cmd="$(_network_find_on_cmdline "${commands}")"
>>  +	if [[ -z "${cmd}" ]]; then
>>  +		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
>>  +		return 0
>>  +	fi
>>  +}
>>   _network_vpn_security_policies() {
>>   	local words=( $@ )
>> 
>>  diff --git a/src/network b/src/network
>>  index 9a2d480..379a71d 100644
>>  --- a/src/network
>>  +++ b/src/network
>>  @@ -1334,6 +1334,9 @@ cli_raw() {
>>   		list-next-free-zones)
>>   			zones_get_next_free
>>   			;;
>>  +		list-valid-ipsec-options)
>>  +			ipsec_get_valid_options "$@"
>>  +			;;
>>   		list-zone-config-ids)
>>   			zone_config_list_ids "$@"
>>   			;;
Hi<br><br>Am Do, 7. Sep, 2017 um 9:45  schrieb Michael Tremer &lt;michael.tremer@ipfire.org&gt;:<br>
<blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">Hi,

technically this work, but is it not better to have a command that checks what
type a connection is (rw or n2n) and then the right list of commands will be
selected?

I think that is cleaner instead of heaving some auto-completion stuff in the
main library.</div></blockquote><div><br></div>IIt may be cleaner but so we only one place where have all possible options listed. This makes adding or changing options much smarter because we have only one place where all options are listed. So I think we should keep this solution and have an n extra variable in the auto-completion wish we have to update on every change.<div><br></div><div>Jonatan&nbsp;<br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">
Best,
-Michael

On Mon, 2017-08-28 at 17:32 +0200, Jonatan Schlag wrote:
<blockquote> The options for the different types of connections should be also printed
 correctly when we use autocompletion
 
 Signed-off-by: Jonatan Schlag &lt;<a href="mailto:jonatan.schlag@ipfire.org">jonatan.schlag@ipfire.org</a>&gt;
 ---
  src/bash-completion/network | 17 ++++++++++++++++-
  src/network                 |  3 +++
  2 files changed, 19 insertions(+), 1 deletion(-)
 
 diff --git a/src/bash-completion/network b/src/bash-completion/network
 index 71bf245..25cf6aa 100644
 --- a/src/bash-completion/network
 +++ b/src/bash-completion/network
 @@ -413,7 +413,7 @@ _network_vpn_ipsec_connection_subcommands() {
  	shift
  	local words=( $@ )
  
 -	local commands="authentication color description down inactivity-
 timeout local mode peer remote security-policy show up"
 +	local commands="$(network raw list-valid-ipsec-options
 "${connection}" "first")"
  	local cmd="$(_network_find_on_cmdline "${commands}")"
  	if [[ -z "${cmd}" ]]; then
  		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
 @@ -432,6 +432,9 @@ _network_vpn_ipsec_connection_subcommands() {
  		description)
  			_network_description ${args}
  			;;
 +		dpd)
 +			_network_vpn_ipsec_connection_subcommands_dpd
 "${connection}" ${args}
 +			;;
  		local)
  			_network_vpn_ipsec_connection_subcommands_local_remot
 e ${connection} "local" ${args}
  			;;
 @@ -529,6 +532,18 @@
 _network_vpn_ipsec_connection_subcommands_security_policy() {
  	fi
  }
  
 +_network_vpn_ipsec_connection_subcommands_dpd() {
 +	local connection=${1}
 +	shift
 +	local words=( $@ )
 +
 +	local commands="$(network raw list-valid-ipsec-options
 "${connection}" "dpd")"
 +	local cmd="$(_network_find_on_cmdline "${commands}")"
 +	if [[ -z "${cmd}" ]]; then
 +		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
 +		return 0
 +	fi
 +}
  _network_vpn_security_policies() {
  	local words=( $@ )
  
 diff --git a/src/network b/src/network
 index 9a2d480..379a71d 100644
 --- a/src/network
 +++ b/src/network
 @@ -1334,6 +1334,9 @@ cli_raw() {
  		list-next-free-zones)
  			zones_get_next_free
  			;;
 +		list-valid-ipsec-options)
 +			ipsec_get_valid_options "$@"
 +			;;
  		list-zone-config-ids)
  			zone_config_list_ids "$@"
  			;;</blockquote></div></blockquote></div>
diff mbox series

Patch

diff --git a/src/bash-completion/network b/src/bash-completion/network
index 71bf245..25cf6aa 100644
--- a/src/bash-completion/network
+++ b/src/bash-completion/network
@@ -413,7 +413,7 @@  _network_vpn_ipsec_connection_subcommands() {
 	shift
 	local words=( $@ )
 
-	local commands="authentication color description down inactivity-timeout local mode peer remote security-policy show up"
+	local commands="$(network raw list-valid-ipsec-options "${connection}" "first")"
 	local cmd="$(_network_find_on_cmdline "${commands}")"
 	if [[ -z "${cmd}" ]]; then
 		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
@@ -432,6 +432,9 @@  _network_vpn_ipsec_connection_subcommands() {
 		description)
 			_network_description ${args}
 			;;
+		dpd)
+			_network_vpn_ipsec_connection_subcommands_dpd "${connection}" ${args}
+			;;
 		local)
 			_network_vpn_ipsec_connection_subcommands_local_remote ${connection} "local" ${args}
 			;;
@@ -529,6 +532,18 @@  _network_vpn_ipsec_connection_subcommands_security_policy() {
 	fi
 }
 
+_network_vpn_ipsec_connection_subcommands_dpd() {
+	local connection=${1}
+	shift
+	local words=( $@ )
+
+	local commands="$(network raw list-valid-ipsec-options "${connection}" "dpd")"
+	local cmd="$(_network_find_on_cmdline "${commands}")"
+	if [[ -z "${cmd}" ]]; then
+		COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )
+		return 0
+	fi
+}
 _network_vpn_security_policies() {
 	local words=( $@ )
 
diff --git a/src/network b/src/network
index 9a2d480..379a71d 100644
--- a/src/network
+++ b/src/network
@@ -1334,6 +1334,9 @@  cli_raw() {
 		list-next-free-zones)
 			zones_get_next_free
 			;;
+		list-valid-ipsec-options)
+			ipsec_get_valid_options "$@"
+			;;
 		list-zone-config-ids)
 			zone_config_list_ids "$@"
 			;;