[Asterisk-code-review] media_cache.c: Various CLI improvements (asterisk[17])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13292 )

Change subject: media_cache.c: Various CLI improvements
......................................................................

media_cache.c: Various CLI improvements

* Use ast_cli_completion_add() to improve performance when large number of
  cached items are present.

* Only complete one URI for commands that only accept a single URI.

* Change command documentation to wrap at 80 characters to improve
  readability.

Change-Id: Iedb0a2c3541e49561bc231dca2dcc0ebd8612902
---
M main/media_cache.c
1 file changed, 19 insertions(+), 28 deletions(-)

Approvals:
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/main/media_cache.c b/main/media_cache.c
index 4493235..15f70d0 100644
--- a/main/media_cache.c
+++ b/main/media_cache.c
@@ -493,8 +493,7 @@
 		e->command = "media cache show all";
 		e->usage =
 			"Usage: media cache show all\n"
-			"       Display a summary of all current items\n"
-			"       in the media cache.\n";
+			"       Display a summary of all current items in the media cache.\n";
 		return NULL;
 	case CLI_GENERATE:
 		return NULL;
@@ -515,27 +514,22 @@
  * \internal
  * \brief CLI tab completion function for URIs
  */
-static char *cli_complete_uri(const char *word, int state)
+static char *cli_complete_uri(const char *word)
 {
 	struct ast_bucket_file *bucket_file;
 	struct ao2_iterator it_media_items;
 	int wordlen = strlen(word);
-	int which = 0;
-	char *result = NULL;
 
 	it_media_items = ao2_iterator_init(media_cache, 0);
 	while ((bucket_file = ao2_iterator_next(&it_media_items))) {
-		if (!strncasecmp(word, ast_sorcery_object_get_id(bucket_file), wordlen)
-			&& ++which > state) {
-			result = ast_strdup(ast_sorcery_object_get_id(bucket_file));
+		if (!strncasecmp(word, ast_sorcery_object_get_id(bucket_file), wordlen)) {
+			ast_cli_completion_add(ast_strdup(ast_sorcery_object_get_id(bucket_file)));
 		}
 		ao2_ref(bucket_file, -1);
-		if (result) {
-			break;
-		}
 	}
 	ao2_iterator_destroy(&it_media_items);
-	return result;
+
+	return NULL;
 }
 
 static char *media_cache_handle_show_item(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
@@ -550,11 +544,10 @@
 		e->command = "media cache show";
 		e->usage =
 			"Usage: media cache show <uri>\n"
-			"       Display all information about a particular\n"
-			"       item in the media cache.\n";
+			"       Display all information about a particular item in the media cache.\n";
 		return NULL;
 	case CLI_GENERATE:
-		return cli_complete_uri(a->word, a->n);
+		return a->pos == e->args ? cli_complete_uri(a->word) : NULL;
 	}
 
 	if (a->argc != 4) {
@@ -590,14 +583,13 @@
 		e->command = "media cache delete";
 		e->usage =
 			"Usage: media cache delete <uri>\n"
-			"       Delete an item from the media cache.\n"
-			"       Note that this will also remove any local\n"
-			"       storage of the media associated with the URI,\n"
-			"       and will inform the backend supporting the URI\n"
-			"       scheme that it should remove the item.\n";
+			"       Delete an item from the media cache.\n\n"
+			"       Note that this will also remove any local storage of the media associated\n"
+			"       with the URI, and will inform the backend supporting the URI scheme that\n"
+			"       it should remove the item.\n";
 		return NULL;
 	case CLI_GENERATE:
-		return cli_complete_uri(a->word, a->n);
+		return a->pos == e->args ? cli_complete_uri(a->word) : NULL;
 	}
 
 	if (a->argc != 4) {
@@ -622,13 +614,12 @@
 		e->command = "media cache refresh";
 		e->usage =
 			"Usage: media cache refresh <uri>\n"
-			"       Ask for a refresh of a particular URI.\n"
-			"       If the item does not already exist in the\n"
-			"       media cache, the item will be populated from\n"
-			"       the backend supporting the URI scheme.\n";
+			"       Ask for a refresh of a particular URI.\n\n"
+			"       If the item does not already exist in the media cache, the item will be\n"
+			"       populated from the backend supporting the URI scheme.\n";
 		return NULL;
 	case CLI_GENERATE:
-		return cli_complete_uri(a->word, a->n);
+		return a->pos == e->args ? cli_complete_uri(a->word) : NULL;
 	}
 
 	if (a->argc != 4) {
@@ -651,8 +642,8 @@
 		e->command = "media cache create";
 		e->usage =
 			"Usage: media cache create <uri> <file>\n"
-			"       Create an item in the media cache by associating\n"
-			"       a local media file with some URI.\n";
+			"       Create an item in the media cache by associating a local media file with\n"
+			"       some URI.\n";
 		return NULL;
 	case CLI_GENERATE:
 		return NULL;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13292
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: Iedb0a2c3541e49561bc231dca2dcc0ebd8612902
Gerrit-Change-Number: 13292
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191202/cc59e6d5/attachment-0001.html>