[Octave-bug-tracker] [bug #65259] Don't say documentation for class itself comes from class constructor

[Rik]

Follow-up Comment #3, bug#65259 (group octave):

This turns out to be pulling on a thread and having the whole sweater
unravel.

I think there is something wrong with the way 'which' is operating on classdef
files.

For starters, the function


  bool get_which_info_from_fcn (const std::string& name, const octave_value&
ov_fcn, std::string& file, std::string& type) const;


is declared in help.h as a private method of the class help_system.  But using
"hg grep" I can't find any other usage of this function outside of help.cc.

1) Wouldn't it be better to make get_which_info_from_fcn a static local
function to help.cc?

The code path begins with


help_system::which (const std::string& name,
                    std::string& type) const


This function first looks for a '.' character to see if it has a whole
function name, or a package/class prefix and a subsequent function name.


  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);

      if (get_which_info_from_fcn (name, ov_fcn, file, type))
        return file;
    }


What's interesting here is that a bare classname, such as "inputParser" runs
through this code path completely missing the next section


  // 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 ();

  // 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);

  if (pkg.ok ())
    {
      // FIXME: How to get the fill name of a package?
      file = pkg.get_name ();
      type = "package";

      return file;
    }


As far as I can tell, this is dead code (well, not the package part).

It seems that either the simple code path should be avoided for a class name
so that the dead code is activated, OR the code in get_which_info_from_fcn()
should be modified to test for bare class name, OR is_classdef_constructor()
should not return true when no input name is given.

Any thoughts, reasoning on which would be better?




    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?65259>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/