* Add hids
@ 2017-07-25 16:07 Jonatan Schlag
2017-07-25 16:07 ` [PATCH 1/8] zone: add config hid functions Jonatan Schlag
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 330 bytes --]
The following patches add support for hids in configs.
Hids are human readable ids.
Please notice that the patches are based on the patchset from the July 24, 2017, 7:10 p.m. beginning with the patch 'zone: add new function zone_config_list_ids' and the following three patches.
These four patches needs to be merged first.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/8] zone: add config hid functions
2017-07-25 16:07 Add hids Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 22:22 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 2/8] zone: accept also hids in zone_config() Jonatan Schlag
` (6 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]
The following patch adds the basic functions which are we need to work with hids.
Fixes: #11406 (partially)
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/functions/functions.zone | 86 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index bba1705..28a22fe 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -656,6 +656,75 @@ zone_config_list_ids() {
echo ${ids}
}
+# List all hids of a zone
+zone_config_list_hids() {
+ assert [ $# -eq 1 ]
+ local zone=${1}
+
+ local config
+ local hids
+
+ for config in $(zone_configs_list ${zone}); do
+ # Get hook from config
+ hook="$(zone_config_get_hook "${zone}" "${config}")"
+ # Append hids to the list
+ list_append hids "$(zone_config_get_hid "${zone}" "${config}")"
+ done
+
+ echo ${hids}
+}
+
+# get the hid from a given config
+zone_config_get_hid() {
+ assert [ $# -eq 2 ]
+ local zone=${1}
+ local config=${2}
+
+ local hook
+
+ hook="$(zone_config_get_hook "${zone}" "${config}")"
+ echo "$(hook_exec "config" "${hook}" "hid" "${zone}" "${config}")"
+}
+
+# Checks if a hid is valid for a given zone
+zone_config_hid_is_valid() {
+ assert [ $# -eq 2]
+ local zone=${1}
+ local hid=${2}
+
+ local _hid
+ for _hid in $(zone_config_list_hids "${zone}"); do
+ if [[ ${_hid} == ${hid} ]]; then
+ return ${EXIT_TRUE}
+ fi
+ done
+
+ return ${EXIT_FALSE}
+
+}
+
+# This function converts a hid to a id
+zone_config_convert_hid_to_id() {
+ assert [ $# -eq 2 ]
+ local zone=${1}
+ local hid=${2}
+
+ local hook
+ local config
+ local id
+
+ for config in $(zone_configs_list ${zone}); do
+ # Get hook from config
+ hook="$(zone_config_get_hook "${zone}" "${config}")"
+ if [[ "$(hook_exec "config" "${hook}" "hid" "${zone}" "${config}")" == "${hid}" ]]; then
+ id=$(config_get_id_from_config "${config}")
+ break
+ fi
+ done
+
+ echo ${id}
+}
+
zone_show() {
local zone=${1}
@@ -1114,6 +1183,23 @@ zone_config_id_is_valid() {
[ -f ${zone_path}/configs/*.${id} ];
}
+# This function checks if a given hid is valid for a zone
+# Return True when yes and false when no
+zone_config_hid_is_valid() {
+ assert [ $# -eq 2 ]
+ local zone=${1}
+ local hid=${2}
+
+ local _hid
+ for _hid in $(zone_config_list_hids ${zone}); do
+ if [[ ${_hid} == ${hid} ]]; then
+ return ${EXIT_TRUE}
+ fi
+ done
+
+ return ${EXIT_FALSE}
+}
+
zone_config_get_hook_from_id() {
# Returns the hook for a given id
assert [ $# -eq 2 ]
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/8] zone: accept also hids in zone_config()
2017-07-25 16:07 Add hids Jonatan Schlag
2017-07-25 16:07 ` [PATCH 1/8] zone: add config hid functions Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 22:27 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 3/8] zone: config list print also hids Jonatan Schlag
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/functions/functions.zone | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index 28a22fe..091bcd6 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -550,28 +550,43 @@ zone_config() {
zone_config_new "${zone}" "$@"
;;
destroy)
+ # id could be also a valid hid
local id=${1}
- if zone_config_id_is_valid ${zone} ${id}; then
+ # usually we will get a hid so we check first if we get one
+ if zone_config_hid_is_valid ${zone} ${id}; then
+ # Convert the hid into an id
+ id=$(zone_config_convert_hid_to_id ${zone} ${id})
zone_config_destroy "${zone}" "$@"
else
- log ERROR "${id} is not a valid id"
+ # We get no valid hid so we check if we get a valid id
+ if zone_config_id_is_valid ${zone} ${id}; then
+ zone_config_destroy "${zone}" "$@"
+ else
+ log ERROR "${id} is not a valid id or hid"
+ fi
fi
;;
list)
zone_config_list "${zone}" "$@"
;;
*)
- # Check is we get a valid id
- # TODO This could be also a valid hid
+ # id could be also a valid hid
local id=${cmd}
-
- if zone_config_id_is_valid ${zone} ${id} && [[ ${1} == "edit" ]]; then
+ # usually we will get a hid so we check first if we get one
+ if zone_config_hid_is_valid ${zone} ${id} && [[ ${1} == "edit" ]]; then
+ id=$(zone_config_convert_hid_to_id ${zone} ${id})
shift 1
zone_config_edit "${zone}" "${id}" "$@"
else
- error "Unrecognized argument: ${cmd}"
- cli_usage root-zone-config-subcommands
- exit ${EXIT_ERROR}
+ # We get no valid hid so we check if we get a valid id
+ if zone_config_id_is_valid ${zone} ${id} && [[ ${1} == "edit" ]]; then
+ shift 1
+ zone_config_edit "${zone}" "${id}" "$@"
+ else
+ error "Unrecognized argument: ${cmd}"
+ cli_usage root-zone-config-subcommands
+ exit ${EXIT_ERROR}
+ fi
fi
;;
esac
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/8] zone: config list print also hids
2017-07-25 16:07 Add hids Jonatan Schlag
2017-07-25 16:07 ` [PATCH 1/8] zone: add config hid functions Jonatan Schlag
2017-07-25 16:07 ` [PATCH 2/8] zone: accept also hids in zone_config() Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 22:28 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 4/8] hook: also hook_hid is a valid command Jonatan Schlag
` (4 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 981 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/functions/functions.zone | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index 091bcd6..17d0858 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -634,21 +634,22 @@ zone_config_list() {
assert isset zone
# Print a nice header
- local format="%-3s %-20s"
- print "${format}" "ID" "HOOK"
+ local format="%-3s %-20s %-20s"
+ print "${format}" "ID" "HOOK" "HID"
local config
local hook
local id
+ local hid
# Print for all config:
# id and hook
- # TODO: Add hids here
for config in $(zone_configs_list "${zone}"); do
id=${config##*.}
hook=$(zone_config_get_hook "${zone}" "${config}")
+ hid=$(zone_config_get_hid "${zone}" "${config}")
assert isset hook
- print "${format}" "${id}" "${hook}"
+ print "${format}" "${id}" "${hook}" "${hid}"
done
}
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/8] hook: also hook_hid is a valid command
2017-07-25 16:07 Add hids Jonatan Schlag
` (2 preceding siblings ...)
2017-07-25 16:07 ` [PATCH 3/8] zone: config list print also hids Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 16:07 ` [PATCH 5/8] header-config: add generic hhok_hid function Jonatan Schlag
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 537 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/functions/functions.hook | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/functions/functions.hook b/src/functions/functions.hook
index d1f6506..d2f4a78 100644
--- a/src/functions/functions.hook
+++ b/src/functions/functions.hook
@@ -204,7 +204,7 @@ hook_valid_command_config() {
local cmd="${1}"
case "${cmd}" in
- new|destroy|edit|up|down|status)
+ new|destroy|edit|up|down|status|hid)
return ${EXIT_TRUE}
;;
esac
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/8] header-config: add generic hhok_hid function
2017-07-25 16:07 Add hids Jonatan Schlag
` (3 preceding siblings ...)
2017-07-25 16:07 ` [PATCH 4/8] hook: also hook_hid is a valid command Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 22:29 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 6/8] raw: add command list-zone-config-hids Jonatan Schlag
` (2 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 673 bytes --]
This function will always be there so when we call hhok_hid we will get a result.
This is also nice for testing.
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
| 8 ++++++++
1 file changed, 8 insertions(+)
--git a/src/header-config b/src/header-config
index e3c6423..76f4bb5 100644
--- a/src/header-config
+++ b/src/header-config
@@ -76,3 +76,11 @@ hook_edit() {
exit ${EXIT_OK}
}
+
+# We will return the id as generic variant of the hid
+# Although this should not be the standard
+hook_hid() {
+ assert [ $# -eq 1 ]
+ local config=${1}
+ echo $(config_get_id_from_config ${config})
+}
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/8] raw: add command list-zone-config-hids
2017-07-25 16:07 Add hids Jonatan Schlag
` (4 preceding siblings ...)
2017-07-25 16:07 ` [PATCH 5/8] header-config: add generic hhok_hid function Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 16:07 ` [PATCH 7/8] raw: add command zone-config-hid-is-valid Jonatan Schlag
2017-07-25 16:07 ` [PATCH 8/8] autocompletion: use hids instead of ids Jonatan Schlag
7 siblings, 0 replies; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 447 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/network | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/network b/src/network
index adc9ac3..7965d62 100644
--- a/src/network
+++ b/src/network
@@ -1380,6 +1380,9 @@ cli_raw() {
list-zone-config-ids)
zone_config_list_ids $@
;;
+ list-zone-config-hids)
+ zone_config_list_hids $@
+ ;;
zone-name-is-valid)
zone_name_is_valid $@
;;
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 7/8] raw: add command zone-config-hid-is-valid
2017-07-25 16:07 Add hids Jonatan Schlag
` (5 preceding siblings ...)
2017-07-25 16:07 ` [PATCH 6/8] raw: add command list-zone-config-hids Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 16:07 ` [PATCH 8/8] autocompletion: use hids instead of ids Jonatan Schlag
7 siblings, 0 replies; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/network | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/network b/src/network
index 7965d62..16c8f60 100644
--- a/src/network
+++ b/src/network
@@ -1389,6 +1389,9 @@ cli_raw() {
zone-config-id-is-valid)
zone_config_id_is_valid $@
;;
+ zone-config-hid-is-valid)
+ zone_config_hid_is_valid $@
+ ;;
*)
error "No such command: ${cmd}"
exit ${EXIT_ERROR}
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 8/8] autocompletion: use hids instead of ids
2017-07-25 16:07 Add hids Jonatan Schlag
` (6 preceding siblings ...)
2017-07-25 16:07 ` [PATCH 7/8] raw: add command zone-config-hid-is-valid Jonatan Schlag
@ 2017-07-25 16:07 ` Jonatan Schlag
2017-07-25 22:30 ` Michael Tremer
7 siblings, 1 reply; 14+ messages in thread
From: Jonatan Schlag @ 2017-07-25 16:07 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/bash-completion/network | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bash-completion/network b/src/bash-completion/network
index 820c2d4..529a772 100644
--- a/src/bash-completion/network
+++ b/src/bash-completion/network
@@ -421,7 +421,7 @@ _network_zone_subcommand_config() {
local words=( $@ )
- local commands="destroy list new $(network raw list-zone-config-ids ${zone})"
+ local commands="destroy list new $(network raw list-zone-config-hids ${zone})"
local cmd="$(_network_find_on_cmdline "${commands}")"
if [[ -z "${cmd}" ]]; then
@@ -442,7 +442,7 @@ _network_zone_subcommand_config() {
*)
# Check if we get a valid id
# TODO: We should also accept a valid hid
- if network raw zone-config-id-is-valid ${zone} ${cmd}; then
+ if network raw zone-config-id-is-valid ${zone} ${cmd} || network raw zone-config-hid-is-valid ${zone} ${cmd}; then
_network_zone_subcommand_config_subcommand ${zone} ${args}
fi
;;
--
2.6.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/8] zone: add config hid functions
2017-07-25 16:07 ` [PATCH 1/8] zone: add config hid functions Jonatan Schlag
@ 2017-07-25 22:22 ` Michael Tremer
0 siblings, 0 replies; 14+ messages in thread
From: Michael Tremer @ 2017-07-25 22:22 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 4045 bytes --]
Hi,
On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote:
> The following patch adds the basic functions which are we need to work with
> hids.
>
> Fixes: #11406 (partially)
>
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/functions/functions.zone | 86
> ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index bba1705..28a22fe 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -656,6 +656,75 @@ zone_config_list_ids() {
> echo ${ids}
> }
>
> +# List all hids of a zone
> +zone_config_list_hids() {
> + assert [ $# -eq 1 ]
> + local zone=${1}
> +
> + local config
> + local hids
> +
> + for config in $(zone_configs_list ${zone}); do
> + # Get hook from config
> + hook="$(zone_config_get_hook "${zone}" "${config}")"
> + # Append hids to the list
> + list_append hids "$(zone_config_get_hid "${zone}"
> "${config}")"
> + done
> +
> + echo ${hids}
> +}
In this function, you are using a hook variable that is never initalised nor
used. So I would say that can just be removed.
And unless you insist that the function returns its result in one line, you
could just let "zone_config_get_hid" write to stdout and remove the hids
variable and list_append entirely. That will make this function way faster since
it doesn't need to fork as many sub-shells.
> +
> +# get the hid from a given config
> +zone_config_get_hid() {
> + assert [ $# -eq 2 ]
> + local zone=${1}
> + local config=${2}
> +
> + local hook
> +
> + hook="$(zone_config_get_hook "${zone}" "${config}")"
> + echo "$(hook_exec "config" "${hook}" "hid" "${zone}" "${config}")"
> +}
Same here:
echo "$(some function)"
could just be:
some function
Same result. But doesn't call echo or the sub-shell to call the function.
> +
> +# Checks if a hid is valid for a given zone
> +zone_config_hid_is_valid() {
> + assert [ $# -eq 2]
> + local zone=${1}
> + local hid=${2}
> +
> + local _hid
> + for _hid in $(zone_config_list_hids "${zone}"); do
> + if [[ ${_hid} == ${hid} ]]; then
> + return ${EXIT_TRUE}
> + fi
> + done
> +
> + return ${EXIT_FALSE}
> +
> +}
Empty line before the closing bracket of the function.
And the check for equality in POSIX shell is just "=" and not "==".
> +
> +# This function converts a hid to a id
> +zone_config_convert_hid_to_id() {
> + assert [ $# -eq 2 ]
> + local zone=${1}
> + local hid=${2}
> +
> + local hook
> + local config
> + local id
> +
> + for config in $(zone_configs_list ${zone}); do
> + # Get hook from config
> + hook="$(zone_config_get_hook "${zone}" "${config}")"
> + if [[ "$(hook_exec "config" "${hook}" "hid" "${zone}"
> "${config}")" == "${hid}" ]]; then
> + id=$(config_get_id_from_config "${config}")
> + break
> + fi
> + done
> +
> + echo ${id}
> +}
Same as above. You could just let "config_get_id_from_config" print the ID and
then break or return. The ID variable is not needed.
> +
> zone_show() {
> local zone=${1}
>
> @@ -1114,6 +1183,23 @@ zone_config_id_is_valid() {
> [ -f ${zone_path}/configs/*.${id} ];
> }
>
> +# This function checks if a given hid is valid for a zone
> +# Return True when yes and false when no
> +zone_config_hid_is_valid() {
> + assert [ $# -eq 2 ]
> + local zone=${1}
> + local hid=${2}
> +
> + local _hid
> + for _hid in $(zone_config_list_hids ${zone}); do
> + if [[ ${_hid} == ${hid} ]]; then
> + return ${EXIT_TRUE}
> + fi
> + done
> +
> + return ${EXIT_FALSE}
> +}
This function is a duplicate.
I also think it should be called "zone_config_hid_exists" or
so since we technically don't know what a valid HID looks like. Every hook can
have something different here.
> +
> zone_config_get_hook_from_id() {
> # Returns the hook for a given id
> assert [ $# -eq 2 ]
Best,
-Michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/8] zone: accept also hids in zone_config()
2017-07-25 16:07 ` [PATCH 2/8] zone: accept also hids in zone_config() Jonatan Schlag
@ 2017-07-25 22:27 ` Michael Tremer
0 siblings, 0 replies; 14+ messages in thread
From: Michael Tremer @ 2017-07-25 22:27 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 3257 bytes --]
Hi,
On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote:
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/functions/functions.zone | 33 ++++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index 28a22fe..091bcd6 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -550,28 +550,43 @@ zone_config() {
> zone_config_new "${zone}" "$@"
> ;;
> destroy)
> + # id could be also a valid hid
> local id=${1}
> - if zone_config_id_is_valid ${zone} ${id}; then
> + # usually we will get a hid so we check first if we
> get one
> + if zone_config_hid_is_valid ${zone} ${id}; then
> + # Convert the hid into an id
> + id=$(zone_config_convert_hid_to_id ${zone}
> ${id})
> zone_config_destroy "${zone}" "$@"
Could we not just call "zone_config_convert_hid_to_id" (maybe the name would be
better without "convert") and if it does not return an ID, we assume that the
HID does not exist?
That would save us running through all the hooks twice which is quite an
expensive thing to do.
> else
> - log ERROR "${id} is not a valid id"
> + # We get no valid hid so we check if we get a
> valid id
> + if zone_config_id_is_valid ${zone} ${id};
> then
> + zone_config_destroy "${zone}" "$@"
> + else
> + log ERROR "${id} is not a valid id or
> hid"
> + fi
> fi
> ;;
> list)
> zone_config_list "${zone}" "$@"
> ;;
> *)
> - # Check is we get a valid id
> - # TODO This could be also a valid hid
> + # id could be also a valid hid
> local id=${cmd}
> -
> - if zone_config_id_is_valid ${zone} ${id} && [[ ${1}
> == "edit" ]]; then
> + # usually we will get a hid so we check first if we
> get one
> + if zone_config_hid_is_valid ${zone} ${id} && [[ ${1}
> == "edit" ]]; then
> + id=$(zone_config_convert_hid_to_id ${zone}
> ${id})
Can we also remove this ${1} == edit thing. That has confused me quite a few
times and doesn't make the code very obvious to read.
But same thing here as above. Could be something like:
local id = $(hid to id ${hid}
if ! isset id; then
... check if HID is an ID
fi
... assume that we have a valid ID from here on ...
This would have a little bit of a performance penalty if the user has given an
ID or an invalid HID. But I think it is rather likely that someone will give a
HID than an ID. Mainly because auto-completion will give a HID instead of an ID.
> shift 1
> zone_config_edit "${zone}" "${id}" "$@"
> else
> - error "Unrecognized argument: ${cmd}"
> - cli_usage root-zone-config-subcommands
> - exit ${EXIT_ERROR}
> + # We get no valid hid so we check if we get a
> valid id
> + if zone_config_id_is_valid ${zone} ${id} &&
> [[ ${1} == "edit" ]]; then
> + shift 1
> + zone_config_edit "${zone}" "${id}"
> "$@"
> + else
> + error "Unrecognized argument: ${cmd}"
> + cli_usage root-zone-config-
> subcommands
> + exit ${EXIT_ERROR}
> + fi
> fi
> ;;
> esac
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/8] zone: config list print also hids
2017-07-25 16:07 ` [PATCH 3/8] zone: config list print also hids Jonatan Schlag
@ 2017-07-25 22:28 ` Michael Tremer
0 siblings, 0 replies; 14+ messages in thread
From: Michael Tremer @ 2017-07-25 22:28 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]
This looks good.
It might be better to print just "-" if there is no HID, but that only makes
parsing this output easier which nobody should be doing any ways...
-Michael
On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote:
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/functions/functions.zone | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index 091bcd6..17d0858 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -634,21 +634,22 @@ zone_config_list() {
> assert isset zone
>
> # Print a nice header
> - local format="%-3s %-20s"
> - print "${format}" "ID" "HOOK"
> + local format="%-3s %-20s %-20s"
> + print "${format}" "ID" "HOOK" "HID"
>
> local config
> local hook
> local id
> + local hid
>
> # Print for all config:
> # id and hook
> - # TODO: Add hids here
> for config in $(zone_configs_list "${zone}"); do
> id=${config##*.}
> hook=$(zone_config_get_hook "${zone}" "${config}")
> + hid=$(zone_config_get_hid "${zone}" "${config}")
> assert isset hook
> - print "${format}" "${id}" "${hook}"
> + print "${format}" "${id}" "${hook}" "${hid}"
> done
> }
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/8] header-config: add generic hhok_hid function
2017-07-25 16:07 ` [PATCH 5/8] header-config: add generic hhok_hid function Jonatan Schlag
@ 2017-07-25 22:29 ` Michael Tremer
0 siblings, 0 replies; 14+ messages in thread
From: Michael Tremer @ 2017-07-25 22:29 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote:
> This function will always be there so when we call hhok_hid we will get a
> result.
> This is also nice for testing.
>
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/header-config | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/src/header-config b/src/header-config
> index e3c6423..76f4bb5 100644
> --- a/src/header-config
> +++ b/src/header-config
> @@ -76,3 +76,11 @@ hook_edit() {
>
> exit ${EXIT_OK}
> }
> +
> +# We will return the id as generic variant of the hid
> +# Although this should not be the standard
> +hook_hid() {
> + assert [ $# -eq 1 ]
> + local config=${1}
> + echo $(config_get_id_from_config ${config})
> +}
Just call "config_get_if_from_config ${config}".
Doesn't need the echo.
Also remove the assert here. The hook should always assume that it is being
called with the correct number of arguments and this will also make auto-
completion slower.
-Michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 8/8] autocompletion: use hids instead of ids
2017-07-25 16:07 ` [PATCH 8/8] autocompletion: use hids instead of ids Jonatan Schlag
@ 2017-07-25 22:30 ` Michael Tremer
0 siblings, 0 replies; 14+ messages in thread
From: Michael Tremer @ 2017-07-25 22:30 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]
On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote:
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/bash-completion/network | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/bash-completion/network b/src/bash-completion/network
> index 820c2d4..529a772 100644
> --- a/src/bash-completion/network
> +++ b/src/bash-completion/network
> @@ -421,7 +421,7 @@ _network_zone_subcommand_config() {
>
> local words=( $@ )
>
> - local commands="destroy list new $(network raw list-zone-config-ids
> ${zone})"
> + local commands="destroy list new $(network raw list-zone-config-hids
> ${zone})"
>
> local cmd="$(_network_find_on_cmdline "${commands}")"
> if [[ -z "${cmd}" ]]; then
> @@ -442,7 +442,7 @@ _network_zone_subcommand_config() {
> *)
> # Check if we get a valid id
> # TODO: We should also accept a valid hid
> - if network raw zone-config-id-is-valid ${zone}
> ${cmd}; then
> + if network raw zone-config-id-is-valid ${zone} ${cmd}
> || network raw zone-config-hid-is-valid ${zone} ${cmd}; then
> _network_zone_subcommand_config_subcommand
> ${zone} ${args}
> fi
> ;;
Again, is it not smarter to assume that a HID has been given instead of an ID?
Saves us a call of the second function...
-Michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-07-25 22:30 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-25 16:07 Add hids Jonatan Schlag
2017-07-25 16:07 ` [PATCH 1/8] zone: add config hid functions Jonatan Schlag
2017-07-25 22:22 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 2/8] zone: accept also hids in zone_config() Jonatan Schlag
2017-07-25 22:27 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 3/8] zone: config list print also hids Jonatan Schlag
2017-07-25 22:28 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 4/8] hook: also hook_hid is a valid command Jonatan Schlag
2017-07-25 16:07 ` [PATCH 5/8] header-config: add generic hhok_hid function Jonatan Schlag
2017-07-25 22:29 ` Michael Tremer
2017-07-25 16:07 ` [PATCH 6/8] raw: add command list-zone-config-hids Jonatan Schlag
2017-07-25 16:07 ` [PATCH 7/8] raw: add command zone-config-hid-is-valid Jonatan Schlag
2017-07-25 16:07 ` [PATCH 8/8] autocompletion: use hids instead of ids Jonatan Schlag
2017-07-25 22:30 ` Michael Tremer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox