The Variable preference implementation have been updated and documented

* If CLIgen matches more than one variable with the same preference, an "Ambiguos command" error is returned * Such tiebreaks can be broken by a new "preference" keyword, eg: `<string preference:20>` * There is also an extended API: `cligen_preference_mode_set()` * Changes to variable preferenes that may effect code: * string variable with expand function has higher preference than without * `Ambiguous command` error is returned in more cases * Described in Section 3.14 of the CLIgen tutorial
clicon · Jul 16, 2022 · 3042f44 · 3042f44
1 parent d8e3586
commit 3042f44
Show file tree

Hide file tree

Showing 15 changed files with 349 additions and 158 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,17 @@
 ## 5.8.0
 Expected: July 2022
 
+### New Features
+
+* The Variable preference implementation have been updated and documented
+  * If CLIgen matches more than one variable with the same preference, an "Ambiguos command" error is returned
+  * Such tiebreaks can be broken by a new "preference" keyword, eg: `<string preference:20>`
+  * There is also an extended API: `cligen_preference_mode_set()`
+  * Changes to variable preferenes that may effect code:
+    * string variable with expand function has higher preference than without
+    * `Ambiguous command` error is returned in more cases
+  * Described in Section 3.14 of the CLIgen tutorial
+
 ### Corrected Bugs
 
 * [Expansion does not work properly for inner hierarchical nodes on pressing TAB](https://github.com/clicon/clixon/issues/332)

diff --git a/cligen_file.c b/cligen_file.c
@@ -203,7 +203,7 @@ usage(char *argv)
 	    "\t-e \t\tSet automatic expansion/completion for all expand() functions\n"
     	    "\t-E \t\tExclude keys in callback cvv. Default include keys\n"
 	    "\t-c \t\tExpand first arg of callback cvv to string matching keywords\n"
-	    "\t-P \t\tSet preference mode to 1, ie return first if several have same pref\n"
+	    "\t-P <mode> \t\tSet preference mode: 1: tiebreak terminals, 2: also non-terminals\n"
 	    "\t-t <nr> \tSet tab mode: 1:columns, 2: same pref for vars, 4: all steps\n"
 	    "\t-s <nr> \tScrolling 0: disable line scrolling, 1: enable line scrolling (default 1)\n"
 	    "\t-u \t\tEnable experimental UTF-8 mode\n"
@@ -267,8 +267,9 @@ main(int   argc,
 	case 'c': /* Expand first arg of callback cvv */
 	    expand_first++;
 	    break;
-	case 'P': /* Return first if several have same preference */
-	    set_preference++;
+	case 'P': /* Return first if several have same preference, for terminals */
+	    argc--;argv++;
+	    set_preference = atoi(*argv);
 	    break;
 	case 'f' : 
 	    argc--;argv++;

diff --git a/cligen_handle.c b/cligen_handle.c
@@ -1031,13 +1031,37 @@ cligen_preference_mode(cligen_handle h)
     return ch->ch_preference_mode;
 }
 
-/*! Set preference mode, return all with same pref(ambiguous) or first (1)
- * More specifically, if several cligen object variables match with same preference,
- * select the first, do not match all.
- * Example:
+/*! Set preference mode, return all with same pref(ambiguous) or first 0-4
+ *
+ * Often, multiple matches will resolve by preference, but not if several matches have same.
+ * This applies to complete (terminal matches), assume length of a is <=4:
+ * @code
  * key (<a:string length[4]> | <a:string length[40]>);
+ * @endcode
+ * Assume the user types the following command which matches both variables:
+ *   key foo
+ * Will lead to: "Ambiguous command"
+ * If set to 1 or 3 will select first variable.
+ *
+ * For non-terminals, example:
+ * @code
+ * key (<a:string length[4]> | <a:string length[40]>){
+ *     port <nr:int32>;
+ * }
+ * @endcode
+ * If set to 2 or 3 will select first variable.
+ *
+ * Do NOT use this if the two variables leads to different choices, eg:
+ * @code
+ * key <a:string length[4]>{
+ *     port <nr:int32>;
+ * }
+ * key <a:string length[40]>){
+ *     description <text:string>;
+ * }
+ * @endcode
  * @param[in] h      CLIgen handle
- * @param[in] flag   Set to 1 to return first, 0 if ambiguous
+ * @param[in] flag   0: ambiguous error, 1: terminal first, 2: non-terminal first match, 3: non-terminal+terminal
  * @retval    0      OK
  */
 int 

diff --git a/cligen_handle.h b/cligen_handle.h
@@ -139,27 +139,27 @@ int   cligen_buf_increase(cligen_handle h, size_t size);
 int   cligen_killbuf_increase(cligen_handle h, size_t size);
 
 /* hack */
-int cligen_parsetree_expand(cligen_handle h, parse_tree ***pt, int **e_len, int **e_i);
+int   cligen_parsetree_expand(cligen_handle h, parse_tree ***pt, int **e_len, int **e_i);
 
-int cligen_lexicalorder(cligen_handle h);
-int cligen_lexicalorder_set(cligen_handle h, int n);
-int cligen_ignorecase(cligen_handle h);
-int cligen_ignorecase_set(cligen_handle h, int n);
+int   cligen_lexicalorder(cligen_handle h);
+int   cligen_lexicalorder_set(cligen_handle h, int n);
+int   cligen_ignorecase(cligen_handle h);
+int   cligen_ignorecase_set(cligen_handle h, int n);
 
-int cligen_logsyntax(cligen_handle h);
-int cligen_logsyntax_set(cligen_handle h, int n);
+int   cligen_logsyntax(cligen_handle h);
+int   cligen_logsyntax_set(cligen_handle h, int n);
 
 void *cligen_userhandle(cligen_handle h);
-int cligen_userhandle_set(cligen_handle h, void *userhandle);
+int   cligen_userhandle_set(cligen_handle h, void *userhandle);
 
-int cligen_regex_xsd(cligen_handle h);
-int cligen_regex_xsd_set(cligen_handle h, int mode);
+int   cligen_regex_xsd(cligen_handle h);
+int   cligen_regex_xsd_set(cligen_handle h, int mode);
 
-char cligen_delimiter(cligen_handle h);
-int  cligen_delimiter_set(cligen_handle h, char delimiter);
+char  cligen_delimiter(cligen_handle h);
+int   cligen_delimiter_set(cligen_handle h, char delimiter);
 
-int cligen_preference_mode(cligen_handle h);
-int cligen_preference_mode_set(cligen_handle h, int flag);
+int   cligen_preference_mode(cligen_handle h);
+int   cligen_preference_mode_set(cligen_handle h, int flag);
 
 int   cligen_caseignore_get(cligen_handle h);
 int   cligen_caseignore_set(cligen_handle h, int ignorecase);

diff --git a/cligen_match.c b/cligen_match.c
@@ -77,10 +77,10 @@
 
 /*! Match variable against input string
  * 
- * @param[in]  string  Input string to match
- * @param[in]  pvt     variable type (from definition)
- * @param[in]  cmd     variable string (from definition) - can contain range
- * 
+ * @param[in]  h       CLIgen handle
+ * @param[in]  co      cligen object
+ * @param[in]  str     Input string to match
+ * @param[out] reason  if not match and co type is 0, reason points to a (malloced) err string
  * @retval     -1      Error (print msg on stderr)
  * @retval     0       Not match and reason returned as malloced string.
  * @retval     1       Match
@@ -116,7 +116,9 @@ match_variable(cligen_handle h,
 }
 
 /*! Given a string and one cligen object, return if the string matches
- * @param[in]  string  Input string to match (NULL is match)
+ *
+ * @param[in]  h       CLIgen handle
+ * @param[in]  str     Input string to match (NULL is match)
  * @param[in]  co      cligen object
  * @param[in]  best    Only return best match (for command evaluation) instead of all possible options
  * @param[out] exact   1 if match is exact (CO_COMMANDS). VARS is 0.
@@ -345,6 +347,7 @@ match_vec(cligen_handle h,
 	  parse_tree   *pt,
 	  char         *token,
 	  char         *resttokens,
+	  int           lasttoken,
 	  int           best,
 	  match_result *mr)
 {
@@ -356,6 +359,7 @@ match_vec(cligen_handle h,
     char   *tmpreason = NULL;
     int     i;
     cg_obj *co;
+    cg_obj *cop;
     int     match;
 
     /* Loop through parse-tree at this level to find matches */
@@ -401,15 +405,31 @@ match_vec(cligen_handle h,
 	    assert(tmpreason == NULL);
 	    if (best){ /* only save best match */
 		if (p == pref_upper){
-		    if (mr_pt_append(mr, co, ISREST(co)?resttokens:token) < 0)
-			goto done;
+		    if (cligen_preference_mode(h) == 1 &&
+			lasttoken &&
+			cop->co_type == CO_VARIABLE && 
+			co->co_type == CO_VARIABLE) /* Skip terminal pref if preference mode */
+			;
+		    else if (cligen_preference_mode(h) == 2 &&
+			!lasttoken &&
+			cop->co_type == CO_VARIABLE && 
+			co->co_type == CO_VARIABLE) /* Skip same pref if preference mode */
+			;
+		    else if (cligen_preference_mode(h) == 3 &&
+			cop->co_type == CO_VARIABLE && 
+			co->co_type == CO_VARIABLE) /* Skip same pref if preference mode */
+			;
+		    else
+			if (mr_pt_append(mr, co, ISREST(co)?resttokens:token) < 0)
+			    goto done;
 		}
 		else if (p > pref_upper){ /* Start again at this level */
 		    pref_upper = p;
 		    if (mr_pt_reset(mr) < 0)
 			goto done;
 		    if (mr_pt_append(mr, co, ISREST(co)?resttokens:token) < 0)
 			goto done;
+		    cop = co;
 		}
 		else{ /* p < pref_upper : skip */
 		}
@@ -528,8 +548,9 @@ match_pattern_sets_local(cligen_handle h,
     /* How many matches of cvt[level+1] in pt */
     if (match_vec(h,
 		  pt, token, resttokens,
+		  lasttoken,
 		  lasttoken?best:1, /* use best preference match in non-terminal matching*/
-		   mr0) < 0)
+		  mr0) < 0)
 	goto done;
     /* Number of matches is 0 (no match), 1 (exact) or many */
     switch (mr_pt_len_get(mr0)){
@@ -556,6 +577,7 @@ match_pattern_sets_local(cligen_handle h,
 	    goto ok; /* will return matches > 1 */ 
 	}
 	mr_pt_reset(mr0);
+	mr_reason_set(mr0, strdup("Ambiguous command"));
 	goto ok;  /* will return matches = 0 */
 	break;
     } /* switch matches */
@@ -885,6 +907,13 @@ match_pattern(cligen_handle h,
      * Good news is that these have been moved from calling functions to here
      * Hopefully it may be easier to simplify
      */
+    /* Intermediate match, have not matched whole command set, stopped short. 
+     * Cases:
+     * 1) Single match: 
+     *      - special case: can accept if REST command, 
+     *      - otherwise set to no match
+     * 2) Multiple match: set no match
+     */
     if (!last_level(cvt, mr_level_get(mr))){ /* XXX level always 0 */
 	if (mr_pt_len_get(mr) == 1){
 	    co_match = mr_pt_i_get(mr, 0);
@@ -901,41 +930,26 @@ match_pattern(cligen_handle h,
 		mr_pt_reset(mr);
 	    }
 	}
-	else {
-	    if ((r = strdup("Unknown command")) == NULL)
-		goto done;
-	    mr_reason_set(mr, r);
-	    mr_pt_reset(mr);
+	else { /* */
+	    if (mr_reason_get(mr) == NULL){
+		if ((r = strdup("Unknown command")) == NULL)
+		    goto done;
+		mr_reason_set(mr, r);
+		mr_pt_reset(mr);
+	    }
 	}
     }
-    /* If more than one choice */
+    /* If multiple match (at final command), collapse to single command if:
+     * 1) All are commands and commands are equal
+     */
     if (mr_pt_len_get(mr) > 1){ 
 	char *string1;
-	int   pref = 0;
 
-	/* Collapse many choices with same pref to one
-	 * if all alternatives are variables with same pref */
-	if (cligen_preference_mode(h)){
-	    for (i=0; i<mr_pt_len_get(mr); i++){
-		co = mr_pt_i_get(mr, i);
-		if (co->co_type != CO_VARIABLE)
-		    break;
-		if (pref == 0)
-		    pref = co_pref(co, 0);
-		else if (pref != co_pref(co, 0))
-		    break;
-	    }
-	    if (i==mr_pt_len_get(mr)){ /* No break in loop: only vraiables with same preference */
-		if (mr_pt_trunc(mr, 1))
-		    goto done;
-	    }
-	}
 	/* Collapse many choices to one
 	 * if all alternatives are equal commands */
 	string1 = NULL;
 	for (i=0; i<mr_pt_len_get(mr); i++){
 	    co = mr_pt_i_get(mr, i);
-	    /* XXX If variable dont compare co_command */
 	    if (co->co_type != CO_COMMAND)
 		break;
 	    if (i == 0)
@@ -952,6 +966,11 @@ match_pattern(cligen_handle h,
 	    if (mr_pt_trunc(mr, 1))
 		goto done;
     }
+    /* Final check:
+     * 1) If no match ensure there is an error message
+     * 2) If single match, ensure there is a NULL child (eg ";" in cligen syntax)
+     *    - if not set to no match
+     */
     switch (mr_pt_len_get(mr)){
     case 0:
 	/* If no match fix an error message */
@@ -968,7 +987,7 @@ match_pattern(cligen_handle h,
 	 * only one sibling to it. */
 	co1 = mr_pt_i_get(mr, 0);
 	/*
-	 * Special case: if a NULL child is not found, then set result == GC_NOMATCH
+	 * Special case: if a NULL child is not found, then set result == CG_NOMATCH
 	 */
 	if ((ptc = co_pt_get(co1)) != NULL && best){
 	    parse_tree *ptn;

diff --git a/cligen_object.c b/cligen_object.c
@@ -40,11 +40,11 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <unistd.h>
+#include <ctype.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
-#include <ctype.h>
 #ifdef HAVE_STRVERSCMP
 #define _GNU_SOURCE
 #define __USE_GNU
@@ -432,7 +432,9 @@ cov_pref(cg_obj *co)
 	pref = 1;
 	break;
     case CGV_STRING:
-	if (co->co_regex)
+	if (co->co_expand_fn_str != NULL)
+	    pref = 8;
+	else if (co->co_regex)
 	    pref = 7;
 	else
 	    pref = 5;
@@ -465,13 +467,14 @@ cov_pref(cg_obj *co)
     case CGV_EMPTY: 
 	break;
     }
+
     return pref;
 }
 
 /*! Assign a preference to a cligen object
  *
  * @param[in]  co    cligen_object
- * @param[in]  exact if match was exact (only applies to CO_COMMAND)
+ * @param[in]  exact if this match is exact (only applies to CO_COMMAND)
  * @retval     pref  Preference: positive integer
  *
  * The higher the better
@@ -486,20 +489,24 @@ co_pref(cg_obj *co,
 {
     int pref = 0;;
 
-    switch (co->co_type){
-    case CO_COMMAND:
-	if (co->co_ref && !exact)
-	    pref = 3; /* expand */
-	else
-	    pref = 100;
-	break;
-    case CO_VARIABLE:
-	pref = cov_pref(co);
-	break;
-    case CO_REFERENCE:
-    case CO_EMPTY:
-	break;
-    }
+    if (co->co_preference > 0)
+	pref = co->co_preference;
+    else
+	switch (co->co_type){
+	case CO_COMMAND:
+	    /* Give full preference to exact command match, low to partial (prefix) command match */
+	    if (exact == 0)
+		pref = 3;
+	    else
+		pref = 100;
+	    break;
+	case CO_VARIABLE:
+	    pref = cov_pref(co);
+	    break;
+	case CO_REFERENCE:
+	case CO_EMPTY:
+	    break;
+	}
     return pref;
 }
 

diff --git a/cligen_object.h b/cligen_object.h
@@ -144,6 +144,7 @@ struct cg_obj{
     struct cg_obj      *co_prev;      /* Parent */
     enum cg_objtype     co_type;      /* Type of object: command, variable or tree
 					 reference */
+    uint16_t            co_preference; /* Overrides default variable preference if != 0*/
     char               *co_command;   /* malloc:ed matching string / name or type */
     char               *co_prefix;    /* Prefix. Can be used in cases where co_command is not unique */
     cg_callback        *co_callbacks; /* linked list of callbacks and arguments */