Re: option for enabling/disabling auto-suggest feature

[Rik-4]

On 06/19/2018 02:21 PM, Sudeepam Pandey wrote:

On Tue, Jun 19, 2018 at 10:24 PM, Rik <[hidden email]> wrote:

On 06/17/2018 11:58 AM, Sudeepam Pandey wrote:

On Wed, May 2, 2018 at 9:34 PM, Rik <[hidden email]> wrote:

On 05/02/2018 05:55 AM, Sudeepam Pandey wrote:

Understood Doug.

There is one last thing left for me to figure out kindly help me with the same.

Like I sad before...I edited the __unimplemented__.m script a little so that, whenever octave fails to find an identifier among the unimplemented functions list as well, it calls my __suggestion__.m script, and this has worked nicely to produce a "Did you mean?" suggestion in the Octave command window in both GUI and CLI.

There are a two things that still need to be taken care of..

1) First of all, we absolutely need a on/off switch for this feature. However, __suggestion__.m will be designed as an internal function (called when an identifier is not found in the unimplemented functions list) so users cannot directly call this function.
So to design a command which can act as a switch for this feature, we can write a m script (say flag.m) which is callable from the command window with a flag value and which would save the desired flag value (indicating on/off) in a .mat file in the "..octave/scripts/help" directory (where __suggestions__.m would reside). The __suggestions__.m script would load this value as soon as it is called and act accordingly. This way the user setting will be saved and will remain protected unless they modify it themselves.
I however, don't understand what files do I have to tweak so that the parser learns to identify this function (flag.m) as a new function of octave, callable from the command window. Also, if we went along with this, what path should I set inside the flag.m file so that the .mat file in "..octave/scripts/help" directory gets updated successfully? considering that the path BEFORE "../octave/scripts/help" would be different for each user, depending on where they have saved their copy of octave.
I understand that I may have said somethings incorrectly here, or that my thinking may be wrong at some places (for example, maybe we have to have a flag.cc file and not a flag.m file) but this approach is probably how a very good on/off feature could be made. Help and suggestions would be appreciated.

I would suggest using getpref()/setpref() for the time being.  For the namespace, use "Octave".  For the actual name of the preference, maybe something like "autosuggest".  In this case, your code can use getpref to check whether the value is boolean true before continuing.  For the time being you don't even need to write an m-file to enable/disable the option.  You can just use setpref directly.  Later on, you could wrap setpref() in an m-file to give it a nice interface.

2) Another thing that bothers me is the absence of missing_property_hook() function...the approach that we have discussed does not work when we make a typo while typing a graphic/line property etc.. We can choose to add a missing_property_hook() function but this would have additional challenges. Examples are...

   a) How do we do it? What files do we tweak?

The file to inspect is libinterp/corefcn/graphics.cc.

grep -n error graphics.cc | grep property

74:  error ("set: invalid value for %s property", pname.c_str ());
104:    error ("%s: unknown %s property %s",
116:      error ("%s: ambiguous %s property name %s; possible matches:\n\n%s",
1312:        error (R"(invalid value for color property "%s")",
1346:              error (e, R"(invalid value for color property "%s" (value = %s))",
1356:        error (R"(invalid value for color property "%s")",
1369:    error (R"(invalid value for color property "%s")",
1384:        error (R"(invalid value for double_radio property "%s")",
1411:    error (R"(invalid value for double_radio property "%s")",
1657:        error (R"(set: invalid graphics handle (= %g) for property "%s")",
1660:        error (R"(set: invalid graphics object type for property "%s")",
1851:        error ("addproperty: missing possible values for radio property");
1913:        error ("addproperty: unsupported type for dynamic property (= %s)",
1937:        error ("addproperty: invalid object type (= %s)",
2194:            error ("invalid %s property '%s'", pfx.c_str (), pname.c_str ());
2219:    error ("invalid default property specification");
3069:              error (e, "error setting default property %s", pname.c_str ());
3098:    error (R"(get: unknown property "%s")", pname.c_str ());
3141:    error (R"(set: unknown property "%s")", pname.c_str ());
3157:    error (R"(get_property: unknown property "%s")", pname.c_str ());
5031:      error ("set: expecting text graphics object or character string for %s property, found %s",
10885:            error ("set: unknown property");
11115:            error ("__go_%s__: missing value for parent property",
11988:    error ("addproperty: invalid graphics object (= %g)", h);
11995:    error ("addproperty: a '%s' property already exists in the graphics object",

Only the error statements which deal with an unknown property would need modification.  I think the idea would be to intercept the error call about an unknown property.  At each point you would first check for missing_property_hook().  If it was empty then error out in the same way as before.  Otherwise, call the function provided in missing property hook and then call error() with whatever text was returned from the suggestions.  Note, that this would just give you suggestions, it would not necessarily be possible to print a menu and have the user decide which one they wanted to change the text to.

All, particularly, Nicholas, Doug, and Rik, I have been able to figure out what error statements would need modifications to make the suggestion feature work for graphic properties. One thing that I could do, is call my suggestion script before the call to the error function and have it return possible correction(s) which could then be passed to the error message to show some suggestions to the users. This, obviously, is an initial thought and the actual UI could be made different.

Regardless of what the UI is made to look like, it will be essential to call the suggestion script (an m-script) from within this c++ file. Could anyone of you explain/point to some article or documentation that explains, how I could call one of my m-scripts from within a c++ file with the incorrect spelling and a flag and have it return some suggestions back? My c++ is not very strong but thankfully, using the above method, a call from this file is all that I would need to do using c++. Every other thing will be handled very well using m-scripts.

As a first pass, I would implement exactly the methodology we have for missing_function_hook.  In pt-id.cc the function is

  void
  tree_identifier::eval_undefined_error (void)
  {
    int l = line ();
    int c = column ();

    maybe_missing_function_hook (name ()); [1]

    if (l == -1 && c == -1)
      error_with_id ("Octave:undefined-function",
                     "'%s' undefined", name ().c_str ());
    else
      error_with_id ("Octave:undefined-function",
                     "'%s' undefined near line %d column %d",
                     name ().c_str (), l, c);
  }

[1]: Thank you for helping Rik, can you please explain the exact purpose of the 'name ()' function here? Is it returning the identifier which was not recognized by the parser? Also, am I correct to think that we are able to directly call the 'maybe_missing_function_hook' here, because we have declared it as a free/global function in the variables.cc file and its header has been included in pt-id.cc? Please correct me if I'm wrong about this.

This was just a rough example of how to create the function.  In your case, change the function prototype to accept the name of the calling function and the unknown property.  For example,

void
err_unknown_property (const std::string& func, const std::string& prop)
{
  maybe_missing_property_hook (prop);

  error_with_id ("Octave:unknown-graphics-property",
                 "%s: unknown graphics property '%s'", func.c_str (), prop.c_str ()));

}

 

The list of calls to error() in graphics.cc above should be examined to find only the ones that elicit "unknown property".  For these errors I would create a local function err_unknown_property().  err_unknown_property would be the analog to eval_undefined_error above.  It would first call maybe_missing_property_hook (name ()) and then call error_with_id().  The function maybe_missing_property_hook should look like maybe_missing_function_hook in variables.cc

I realize that we have discussed the idea of a missing_property_hook() before as well (in this very thread). I do understand the flow that you have mentioned above and honestly, I have been trying to implement that. The thing is, I do not understand the code of missing_function_hook() very well and this is what has been preventing me to code up a missing_property_hook() that is exactly like the missing_function_hook().

For example, I do understand that....

          // Call.
          octave::feval (func_name, octave_value (name));

is somehow calling the m-script that the missing_function_hook points to, but I am not sure why this particular line has been written this way.

Since your project is really about the suggestion engine, I say that it is okay in this instance not to fully understand this bit of C++ code and instead just use it.

The same can be said for other lines that are present in the two code snippets included below.

void
maybe_missing_function_hook (const std::string& name)
{
  // Don't do this if we're handling errors.
  if (buffer_error_messages == 0 && ! Vmissing_function_hook.empty ())
    {
      octave::symbol_table& symtab
        = octave::__get_symbol_table__ ("maybe_missing_function_hook");

      octave_value val = symtab.find_function (Vmissing_function_hook);

      if (val.is_defined ())
        {
          // Ensure auto-restoration.
          octave::unwind_protect frame;
          frame.protect_var (Vmissing_function_hook);

          // Clear the variable prior to calling the function.
          const std::string func_name = Vmissing_function_hook;
          Vmissing_function_hook.clear ();

          // Call.
          octave::feval (func_name, octave_value (name));
        }
    }
} 

 

You will also need something like this from variables.cc adapted to graphics.cc

static std::string Vmissing_function_hook = "__unimplemented__";

DEFUN (missing_function_hook, args, nargout,
       doc: /* -*- texinfo -*-
@deftypefn  {} {@var{val} =} missing_function_hook ()
@deftypefnx {} {@var{old_val} =} missing_function_hook (@var{new_val})
@deftypefnx {} {} missing_function_hook (@var{new_val}, "local")
Query or set the internal variable that specifies the function to call when
an unknown identifier is requested.

When called from inside a function with the @qcode{"local"} option, the
variable is changed locally for the function and any subroutines it calls.
The original variable value is restored when exiting the function.
@seealso{missing_component_hook}
@end deftypefn */)
{
  return SET_INTERNAL_VARIABLE (missing_function_hook);
}

Hope this helps,

Rik

As a solution, maybe I could code the local function err_unknown_property () and have it call the function missing_property_hook (), as discussed. However, for now, the only thing that missing_property_hook() would do, is trigger the suggestion feature by calling my __suggestions__.m script that displays the possible corrections for the misspelled identifiers. No auto-restoration (see code) etc. Maybe we can include a FIXME note with this and later on, someone who understands the code of missing_function_hook() very well could make the missing_property_hook() look exactly like the missing_function_hook().

However, I am willing to code an exact missing_property_hook() myself, but like I said, I would not be able to do so without some help. I would be thankful and would do the exact code if you or anyone else,

- could tell me how I should work my way to understand these two pieces of code,

- or, could explain the code of missing_function_hook() in detail,
- or maybe tell me what lines would be exactly the same and what lines would be modified and how they are modified.

Take the code snippets I extracted and do a search and replace "s/missing_function_hook/missing_property_hook/g".  You may have to add a header file or two if graphics.cc does not compile, but that should be it.

--Rik

Thankyou,

Sudeepam

   b) If we finally make it, what does the parser prioritize when it doesn't recognize an identifier? missing_function_hook or the missing_property_hook?

If you implement it the way I suggested then the two functions are at different levels of parsing and missing_function_hook would take precedence over missing_property_hook.

...and maybe more challenges when we start thinking how?

I really would like to include properties as well but I'm not sure if they could be done right now. Can anyone help with this?

--P Sudeepam

[Sudeepam Pandey]

On Wed, 20 Jun 2018, 11:09 p.m. Rik, <[hidden email]> wrote:

On 06/19/2018 02:21 PM, Sudeepam Pandey wrote:

On Tue, Jun 19, 2018 at 10:24 PM, Rik <[hidden email]> wrote:

On 06/17/2018 11:58 AM, Sudeepam Pandey wrote:

On Wed, May 2, 2018 at 9:34 PM, Rik <[hidden email]> wrote:

On 05/02/2018 05:55 AM, Sudeepam Pandey wrote:

Understood Doug.

There is one last thing left for me to figure out kindly help me with the same.

Like I sad before...I edited the __unimplemented__.m script a little so that, whenever octave fails to find an identifier among the unimplemented functions list as well, it calls my __suggestion__.m script, and this has worked nicely to produce a "Did you mean?" suggestion in the Octave command window in both GUI and CLI.

There are a two things that still need to be taken care of..

1) First of all, we absolutely need a on/off switch for this feature. However, __suggestion__.m will be designed as an internal function (called when an identifier is not found in the unimplemented functions list) so users cannot directly call this function.
So to design a command which can act as a switch for this feature, we can write a m script (say flag.m) which is callable from the command window with a flag value and which would save the desired flag value (indicating on/off) in a .mat file in the "..octave/scripts/help" directory (where __suggestions__.m would reside). The __suggestions__.m script would load this value as soon as it is called and act accordingly. This way the user setting will be saved and will remain protected unless they modify it themselves.
I however, don't understand what files do I have to tweak so that the parser learns to identify this function (flag.m) as a new function of octave, callable from the command window. Also, if we went along with this, what path should I set inside the flag.m file so that the .mat file in "..octave/scripts/help" directory gets updated successfully? considering that the path BEFORE "../octave/scripts/help" would be different for each user, depending on where they have saved their copy of octave.
I understand that I may have said somethings incorrectly here, or that my thinking may be wrong at some places (for example, maybe we have to have a flag.cc file and not a flag.m file) but this approach is probably how a very good on/off feature could be made. Help and suggestions would be appreciated.

I would suggest using getpref()/setpref() for the time being.  For the namespace, use "Octave".  For the actual name of the preference, maybe something like "autosuggest".  In this case, your code can use getpref to check whether the value is boolean true before continuing.  For the time being you don't even need to write an m-file to enable/disable the option.  You can just use setpref directly.  Later on, you could wrap setpref() in an m-file to give it a nice interface.

2) Another thing that bothers me is the absence of missing_property_hook() function...the approach that we have discussed does not work when we make a typo while typing a graphic/line property etc.. We can choose to add a missing_property_hook() function but this would have additional challenges. Examples are...

   a) How do we do it? What files do we tweak?

The file to inspect is libinterp/corefcn/graphics.cc.

grep -n error graphics.cc | grep property

74:  error ("set: invalid value for %s property", pname.c_str ());
104:    error ("%s: unknown %s property %s",
116:      error ("%s: ambiguous %s property name %s; possible matches:\n\n%s",
1312:        error (R"(invalid value for color property "%s")",
1346:              error (e, R"(invalid value for color property "%s" (value = %s))",
1356:        error (R"(invalid value for color property "%s")",
1369:    error (R"(invalid value for color property "%s")",
1384:        error (R"(invalid value for double_radio property "%s")",
1411:    error (R"(invalid value for double_radio property "%s")",
1657:        error (R"(set: invalid graphics handle (= %g) for property "%s")",
1660:        error (R"(set: invalid graphics object type for property "%s")",
1851:        error ("addproperty: missing possible values for radio property");
1913:        error ("addproperty: unsupported type for dynamic property (= %s)",
1937:        error ("addproperty: invalid object type (= %s)",
2194:            error ("invalid %s property '%s'", pfx.c_str (), pname.c_str ());
2219:    error ("invalid default property specification");
3069:              error (e, "error setting default property %s", pname.c_str ());
3098:    error (R"(get: unknown property "%s")", pname.c_str ());
3141:    error (R"(set: unknown property "%s")", pname.c_str ());
3157:    error (R"(get_property: unknown property "%s")", pname.c_str ());
5031:      error ("set: expecting text graphics object or character string for %s property, found %s",
10885:            error ("set: unknown property");
11115:            error ("__go_%s__: missing value for parent property",
11988:    error ("addproperty: invalid graphics object (= %g)", h);
11995:    error ("addproperty: a '%s' property already exists in the graphics object",

Only the error statements which deal with an unknown property would need modification.  I think the idea would be to intercept the error call about an unknown property.  At each point you would first check for missing_property_hook().  If it was empty then error out in the same way as before.  Otherwise, call the function provided in missing property hook and then call error() with whatever text was returned from the suggestions.  Note, that this would just give you suggestions, it would not necessarily be possible to print a menu and have the user decide which one they wanted to change the text to.

All, particularly, Nicholas, Doug, and Rik, I have been able to figure out what error statements would need modifications to make the suggestion feature work for graphic properties. One thing that I could do, is call my suggestion script before the call to the error function and have it return possible correction(s) which could then be passed to the error message to show some suggestions to the users. This, obviously, is an initial thought and the actual UI could be made different.

Regardless of what the UI is made to look like, it will be essential to call the suggestion script (an m-script) from within this c++ file. Could anyone of you explain/point to some article or documentation that explains, how I could call one of my m-scripts from within a c++ file with the incorrect spelling and a flag and have it return some suggestions back? My c++ is not very strong but thankfully, using the above method, a call from this file is all that I would need to do using c++. Every other thing will be handled very well using m-scripts.

As a first pass, I would implement exactly the methodology we have for missing_function_hook.  In pt-id.cc the function is

  void
  tree_identifier::eval_undefined_error (void)
  {
    int l = line ();
    int c = column ();

    maybe_missing_function_hook (name ()); [1]

    if (l == -1 && c == -1)
      error_with_id ("Octave:undefined-function",
                     "'%s' undefined", name ().c_str ());
    else
      error_with_id ("Octave:undefined-function",
                     "'%s' undefined near line %d column %d",
                     name ().c_str (), l, c);
  }

[1]: Thank you for helping Rik, can you please explain the exact purpose of the 'name ()' function here? Is it returning the identifier which was not recognized by the parser? Also, am I correct to think that we are able to directly call the 'maybe_missing_function_hook' here, because we have declared it as a free/global function in the variables.cc file and its header has been included in pt-id.cc? Please correct me if I'm wrong about this.

This was just a rough example of how to create the function.  In your case, change the function prototype to accept the name of the calling function and the unknown property.  For example,

void
err_unknown_property (const std::string& func, const std::string& prop)
{
  maybe_missing_property_hook (prop);

  error_with_id ("Octave:unknown-graphics-property",
                 "%s: unknown graphics property '%s'", func.c_str (), prop.c_str ()));

}

 

The list of calls to error() in graphics.cc above should be examined to find only the ones that elicit "unknown property".  For these errors I would create a local function err_unknown_property().  err_unknown_property would be the analog to eval_undefined_error above.  It would first call maybe_missing_property_hook (name ()) and then call error_with_id().  The function maybe_missing_property_hook should look like maybe_missing_function_hook in variables.cc

I realize that we have discussed the idea of a missing_property_hook() before as well (in this very thread). I do understand the flow that you have mentioned above and honestly, I have been trying to implement that. The thing is, I do not understand the code of missing_function_hook() very well and this is what has been preventing me to code up a missing_property_hook() that is exactly like the missing_function_hook().

For example, I do understand that....

          // Call.
          octave::feval (func_name, octave_value (name));

is somehow calling the m-script that the missing_function_hook points to, but I am not sure why this particular line has been written this way.

Since your project is really about the suggestion engine, I say that it is okay in this instance not to fully understand this bit of C++ code and instead just use it.

The same can be said for other lines that are present in the two code snippets included below.

void
maybe_missing_function_hook (const std::string& name)
{
  // Don't do this if we're handling errors.
  if (buffer_error_messages == 0 && ! Vmissing_function_hook.empty ())
    {
      octave::symbol_table& symtab
        = octave::__get_symbol_table__ ("maybe_missing_function_hook");

      octave_value val = symtab.find_function (Vmissing_function_hook);

      if (val.is_defined ())
        {
          // Ensure auto-restoration.
          octave::unwind_protect frame;
          frame.protect_var (Vmissing_function_hook);

          // Clear the variable prior to calling the function.
          const std::string func_name = Vmissing_function_hook;
          Vmissing_function_hook.clear ();

          // Call.
          octave::feval (func_name, octave_value (name));
        }
    }
} 

 

You will also need something like this from variables.cc adapted to graphics.cc

static std::string Vmissing_function_hook = "__unimplemented__";

DEFUN (missing_function_hook, args, nargout,
       doc: /* -*- texinfo -*-
@deftypefn  {} {@var{val} =} missing_function_hook ()
@deftypefnx {} {@var{old_val} =} missing_function_hook (@var{new_val})
@deftypefnx {} {} missing_function_hook (@var{new_val}, "local")
Query or set the internal variable that specifies the function to call when
an unknown identifier is requested.

When called from inside a function with the @qcode{"local"} option, the
variable is changed locally for the function and any subroutines it calls.
The original variable value is restored when exiting the function.
@seealso{missing_component_hook}
@end deftypefn */)
{
  return SET_INTERNAL_VARIABLE (missing_function_hook);
}

Hope this helps,

Rik

As a solution, maybe I could code the local function err_unknown_property () and have it call the function missing_property_hook (), as discussed. However, for now, the only thing that missing_property_hook() would do, is trigger the suggestion feature by calling my __suggestions__.m script that displays the possible corrections for the misspelled identifiers. No auto-restoration (see code) etc. Maybe we can include a FIXME note with this and later on, someone who understands the code of missing_function_hook() very well could make the missing_property_hook() look exactly like the missing_function_hook().

However, I am willing to code an exact missing_property_hook() myself, but like I said, I would not be able to do so without some help. I would be thankful and would do the exact code if you or anyone else,

- could tell me how I should work my way to understand these two pieces of code,

- or, could explain the code of missing_function_hook() in detail,
- or maybe tell me what lines would be exactly the same and what lines would be modified and how they are modified.

Take the code snippets I extracted and do a search and replace "s/missing_function_hook/missing_property_hook/g".  You may have to add a header file or two if graphics.cc does not compile, but that should be it.

--Rik

Thank you for helping Rik. I'll work on it and reach out if I get stuck again.

-Sudeepam

Thankyou,

Sudeepam

   b) If we finally make it, what does the parser prioritize when it doesn't recognize an identifier? missing_function_hook or the missing_property_hook?

If you implement it the way I suggested then the two functions are at different levels of parsing and missing_function_hook would take precedence over missing_property_hook.

...and maybe more challenges when we start thinking how?

I really would like to include properties as well but I'm not sure if they could be done right now. Can anyone help with this?

--P Sudeepam