Mercurial > octave
changeset 32893:2faec354b977 stable
improve symbol lookup in help and which functions (bug #65220)
* help.h, help.cc (get_help_from_fcn): New static function.
(help_system::get_which_info_from_fcn): New member function.
(help_system::which, help_system::raw_help_from_symbol_table):
Improve symbol lookup for names like "a.b.c". Consider packages,
classes, methods, and properties.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Mon, 29 Jan 2024 00:08:00 -0500 |
parents | 80d3a6abfc4d |
children | 9104b087dc26 82b67791bf1b |
files | libinterp/corefcn/help.cc libinterp/corefcn/help.h |
diffstat | 2 files changed, 284 insertions(+), 50 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/help.cc Wed Jan 31 23:19:10 2024 -0500 +++ b/libinterp/corefcn/help.cc Mon Jan 29 00:08:00 2024 -0500 @@ -232,24 +232,15 @@ return h; } -std::string -help_system::which (const std::string& name, - std::string& type) const +bool +help_system::get_which_info_from_fcn (const std::string& name, const octave_value& ov_fcn, std::string& file, std::string& type) const { - std::string file; - - if (name.empty ()) - return file; - + file = ""; type = ""; - symbol_table& symtab = m_interpreter.get_symbol_table (); - - octave_value val = symtab.find_function (name); - - if (val.is_defined ()) + if (ov_fcn.is_function ()) { - octave_function *fcn = val.function_value (); + octave_function *fcn = ov_fcn.function_value (); if (fcn) { @@ -269,11 +260,10 @@ } else { - file = fcn->fcn_file_name (); if (! file.empty ()) - type = val.is_user_script () ? "script" : "function"; + type = ov_fcn.is_user_script () ? "script" : "function"; else { if (fcn->is_user_function ()) @@ -285,6 +275,8 @@ } } } + + return true; } else { @@ -293,24 +285,147 @@ load_path& lp = m_interpreter.get_load_path (); file = lp.find_fcn_file (name); + + if (file.empty ()) + return false; } } - if (file.empty ()) + return false; +} + +// FIXME: There is a lot of duplication between the following function +// and help_system::raw_help_from_symbol_table. Some refactoring would +// probably be useful. + +std::string +help_system::which (const std::string& name, + std::string& type) const +{ + std::string file; + + if (name.empty ()) + return file; + + type = ""; + + symbol_table& symtab = m_interpreter.get_symbol_table (); + + size_t pos = name.find ('.'); + + if (pos == std::string::npos) { - // File query. + // Simple name. If not found, continue looking for packages and + // classes. + + octave_value ov_fcn = symtab.find_function (name); + + if (get_which_info_from_fcn (name, ov_fcn, file, type)) + return file; + } + + // NAME contains '.' and must match the following pattern: + // + // (package.)*(package|classname).(property|method)* + // + // Start by looking up the full name. It could be either a package or + // a class and we are done. Otherwise, strip the final component and + // lookup that name. If it is a package, look for a function. If it + // is a class, look for a property or method. + + cdef_manager& cdm = m_interpreter.get_cdef_manager (); - load_path& lp = m_interpreter.get_load_path (); + // FIXME: In the following search we may load classes. Is that really + // what we want, or should we just search the loadpath for + // +pkga/+pkgb/classname/file.m, etc. and attempt to extract help text + // without actually installing packages and classes into the fcn_info + // table? + + // Is NAME a class? + + cdef_class cls = cdm.find_class (name, false, true); + + if (cls.ok ()) + { + // FIXME: Return documentation for the class or the class + // constructor? + + file = cls.file_name (); + type = "classdef class"; + + return file; + } + + cdef_package pkg = cdm.find_package (name, false, true); - // For compatibility: "file." queries "file". - if (name.size () > 1 && name[name.size () - 1] == '.') - file = lp.find_file (name.substr (0, name.size () - 1)); - else - file = lp.find_file (name); + if (pkg.ok ()) + { + // FIXME: How to get the fill name of a package? + file = pkg.get_name (); + type = "package"; + + return file; + } + + // Strip final component (might be a property or method name). + + pos = name.rfind ('.'); + std::string prefix = name.substr (0, pos); + std::string nm = name.substr (pos+1); + + // Is PREFIX the name of a class? + + cls = cdm.find_class (prefix, false, true); + + if (cls.ok ()) + { + // FIXME: Should we only find public methods here? + + octave_value ov_meth = cls.get_method (nm); + + if (get_which_info_from_fcn (nm, ov_meth, file, type)) + return file; + + // FIXME: Should we only find public properties here? + + cdef_property prop = cls.find_property (nm); - file = sys::env::make_absolute (file); + if (prop.ok ()) + { + file = cls.file_name (); + type = "class property"; + + return file; + } } + // Or is PREFIX the name of a package? + + pkg = cdm.find_package (prefix, false, true); + + if (pkg.ok ()) + { + octave_value ov_fcn = pkg.find (nm); + + if (get_which_info_from_fcn (nm, ov_fcn, file, type)) + return file; + } + + // File query. + + load_path& lp = m_interpreter.get_load_path (); + + // For compatibility: "file." queries "file". + if (name.size () > 1 && name[name.size () - 1] == '.') + file = lp.find_file (name.substr (0, name.size () - 1)); + else + file = lp.find_file (name); + + file = sys::env::make_absolute (file); + + if (! file.empty ()) + type = "file"; + return file; } @@ -545,51 +660,168 @@ return retval; } +static bool +get_help_from_fcn (const std::string& fcn_nm, const octave_value& ov_fcn, std::string& help, std::string& what, bool& symbol_found) +{ + symbol_found = false; + + help = ""; + what = ""; + + if (ov_fcn.is_function ()) + { + octave_function *fcn = ov_fcn.function_value (); + + help = fcn->doc_string (fcn_nm); + what = fcn->fcn_file_name (); + + if (what.empty ()) + what = fcn->is_user_function () ? "command-line function" : "built-in function"; + + symbol_found = true; + } + + return symbol_found; +} + +// FIXME: There is a lot of duplication between the following function +// and help_system::which. Some refactoring would probably be useful. + bool -help_system::raw_help_from_symbol_table (const std::string& nm, - std::string& h, std::string& w, - bool& symbol_found) const +help_system::raw_help_from_symbol_table (const std::string& name, std::string& help, std::string& what, bool& symbol_found) const { - std::string meth_nm; - symbol_table& symtab = m_interpreter.get_symbol_table (); - octave_value val = symtab.find_function (nm); + size_t pos = name.find ('.'); + + if (pos == std::string::npos) + { + // Simple name. If not found, continue looking for packages and + // classes. + + octave_value ov_fcn = symtab.find_function (name); + + // FIXME: it seems like there is a lot of potential for confusion + // because is_function can also return true for + // octave_classdef_meta objects. - if (! val.is_defined ()) - { - std::size_t pos = nm.rfind ('.'); + if (! ov_fcn.is_classdef_meta () + && get_help_from_fcn (name, ov_fcn, help, what, symbol_found)) + return true; + } + + // If NAME does not contain '.', then it should be a package or a + // class name. + // + // If NAME contains '.' it should match the following pattern: + // + // (package.)*(package|classname).(property|method)* + // + // Start by looking up the full name. It could be either a package or + // a class and we are done. Otherwise, strip the final component and + // lookup that name. If it is a package, look for a function. If it + // is a class, look for a property or method. + + cdef_manager& cdm = m_interpreter.get_cdef_manager (); - if (pos != std::string::npos) + // FIXME: In the following search we may load classes. Is that really + // what we want, or should we just search the loadpath for + // +pkga/+pkgb/classname/file.m, etc. and attempt to extract help text + // without actually installing packages and classes into the fcn_info + // table? + + // Is NAME a class? + + cdef_class cls = cdm.find_class (name, false, true); + + if (cls.ok ()) + { + // Is the class documented? + + help = cls.doc_string (); + + if (! help.empty ()) { - meth_nm = nm.substr (pos+1); + what = "class"; + + symbol_found = true; + return true; + } + + // Look for constructor. - val = symtab.find_function (nm.substr (0, pos)); + pos = name.rfind ('.'); + std::string nm = name.substr (pos+1); + + octave_value ov_meth = cls.get_method (nm); + + if (get_help_from_fcn (nm, ov_meth, help, what, symbol_found)) + { + what = "constructor"; + + return true; } } - if (val.is_defined ()) + cdef_package pkg = cdm.find_package (name, false, true); + + if (pkg.ok ()) { - octave_function *fcn = val.function_value (); + help = "package " + name; + what = "package"; + + symbol_found = true; + return true; + } + + // Strip final component (might be a property or method name). + + pos = name.rfind ('.'); + std::string prefix = name.substr (0, pos); + std::string nm = name.substr (pos+1); + + // Is PREFIX the name of a class? + + cls = cdm.find_class (prefix, false, true); - if (fcn) + if (cls.ok ()) + { + // FIXME: Should we only find public methods here? + + octave_value ov_meth = cls.get_method (nm); + + if (get_help_from_fcn (nm, ov_meth, help, what, symbol_found)) + return true; + + // FIXME: Should we only find public properties here? + + cdef_property prop = cls.find_property (nm); + + if (prop.ok ()) { - // FCN may actually be a classdef_meta object. + // FIXME: is it supposed to be possible to document + // properties? + + help = prop.doc_string (); + what = "class property"; symbol_found = true; - - h = fcn->doc_string (meth_nm); - - w = fcn->fcn_file_name (); - - if (w.empty ()) - w = fcn->is_user_function () ? "command-line function" - : "built-in function"; - return true; } } + // Or is PREFIX the name of a package? + + pkg = cdm.find_package (prefix, false, true); + + if (pkg.ok ()) + { + octave_value ov_fcn = pkg.find (nm); + + if (get_help_from_fcn (nm, ov_fcn, help, what, symbol_found)) + return true; + } + return false; }
--- a/libinterp/corefcn/help.h Wed Jan 31 23:19:10 2024 -0500 +++ b/libinterp/corefcn/help.h Mon Jan 29 00:08:00 2024 -0500 @@ -187,6 +187,8 @@ return old_val; } + bool get_which_info_from_fcn (const std::string& name, const octave_value& ov_fcn, std::string& file, std::string& type) const; + string_vector local_functions () const; bool raw_help_from_symbol_table (const std::string& nm,