help as m-files

[Søren Hauberg]

Hi All,
  Attached is a fairly large changeset (hence the need for compressing
it) that re-implements the help system as m-files. This is pretty much a
fresh implementation, so there will be bugs (although I've removed those
I've been able to find). So, why would we want to re-implement this as
m-files, when we have something that's already functional?

  * It simplifies the code quite a bit, which should make it easier for
    new people to hack on. Especially, the 'lookfor' implementation is
    a lot more simple.

  * It provides some new functionality to Octave. Specifically, it is
    now quite easy to get the help text of a function, and run it
    through 'makeinfo'. This makes it fairly simple to generate html
    versions of the help texts.

  * 'lookfor' is now very fast. The new implementation of 'lookfor' can
    use cached help texts, which makes it very fast. For me, a call to
    'lookfor' usually takes less than a second (compared ~1 minute
    before).

The patch is, however, not entirely complete. To generate caches for
'lookfor', you need to run the 'gen_doc_cache' function. Basically,
'gen_doc_cache' traverses the current path, and creates a cache file in
each directory. This should either happen during 'make' or 'make
install'. I'm not sure which is better, so I'd appreciate some help
here. Also, this should happen during 'pkg install' such that the user
can search package help texts fast.

I also have the following on my outstanding-issues-list:

  * Perhaps move internal variables (eg 'makeinfo_program') to m-files?
 
  * how should 'private' directories be treated in Makefiles ?
 
  * Fix XXX's in code (nothing major).

Søren

[John W. Eaton]

On 29-Oct-2008, Søren Hauberg wrote:

| Hi All,
|   Attached is a fairly large changeset (hence the need for compressing
| it) that re-implements the help system as m-files. This is pretty much a
| fresh implementation, so there will be bugs (although I've removed those
| I've been able to find). So, why would we want to re-implement this as
| m-files, when we have something that's already functional?
|
|   * It simplifies the code quite a bit, which should make it easier for
|     new people to hack on. Especially, the 'lookfor' implementation is
|     a lot more simple.
|
|   * It provides some new functionality to Octave. Specifically, it is
|     now quite easy to get the help text of a function, and run it
|     through 'makeinfo'. This makes it fairly simple to generate html
|     versions of the help texts.
|
|   * 'lookfor' is now very fast. The new implementation of 'lookfor' can
|     use cached help texts, which makes it very fast. For me, a call to
|     'lookfor' usually takes less than a second (compared ~1 minute
|     before).
|
| The patch is, however, not entirely complete. To generate caches for
| 'lookfor', you need to run the 'gen_doc_cache' function. Basically,
| 'gen_doc_cache' traverses the current path, and creates a cache file in
| each directory. This should either happen during 'make' or 'make
| install'. I'm not sure which is better, so I'd appreciate some help
| here. Also, this should happen during 'pkg install' such that the user
| can search package help texts fast.

OK, I'm generally in favor of this change as I'd like to see more
things implemented in .m files if possible.

Is lookfor faster only because of the switch to using a cache?  If
there is no cache, is it still slow?

Could we just check to ensure that the cache is up to date when adding
directories to the load path?  I'm assuming you could do this by
comparing the mtime of the cache file with the mtimes of .m files in the
directory.  I think we already call stat on every file in each load
path directory when we add the directory to the load path, so this
additional check should not add much to Octave's startup time.

Then later, when running lookfor, would you also want to ensure that the
cache files are up to date, or would you just assume that they are?
It seems it would be best to check timestamps, but that could slow
things down again

| I also have the following on my outstanding-issues-list:
|
|   * Perhaps move internal variables (eg 'makeinfo_program') to m-files?

Yes, I think that would be good to do.

|   * how should 'private' directories be treated in Makefiles ?

I'm not sure, but we do need to do something for this since there are
other places where we could use private functions.

|   * Fix XXX's in code (nothing major).

Please use FIXME instead of XXX as people have complained in the past
about patches/mail with XXX being flagged as spam.

jwe

[Søren Hauberg]

ons, 29 10 2008 kl. 14:30 -0400, skrev John W. Eaton:
> OK, I'm generally in favor of this change as I'd like to see more
> things implemented in .m files if possible.
>
> Is lookfor faster only because of the switch to using a cache?  If
> there is no cache, is it still slow?

I think it's comparable to the current implementation, when there is no
cache. The slow part of lookfor is that we have to call makeinfo for
every single function. The fact that this happens in an interpreted
for-loop instead of a compiled one doesn't make much of a difference.

So, the speedup is definitely due to the use of caches.

> Could we just check to ensure that the cache is up to date when adding
> directories to the load path?  I'm assuming you could do this by
> comparing the mtime of the cache file with the mtimes of .m files in the
> directory.  I think we already call stat on every file in each load
> path directory when we add the directory to the load path, so this
> additional check should not add much to Octave's startup time.

What would you do if the cache is out of date? Delete the cache?
Recompute it? I imagine that caches should only be used with the
functions that come with Octave (and hence shouldn't change), and
functions installed with the package manager (and hence shouldn't
change). It should be said, that my implementation of 'lookfor'
essentially works like this

  for_each_dir_in_path:
    if (dir_has_cache)
      search_cache ()
    else
      search_file_by_file ()

So, things work just fine if some directories are cached, while others
are not.

> Then later, when running lookfor, would you also want to ensure that the
> cache files are up to date, or would you just assume that they are?
> It seems it would be best to check timestamps, but that could slow
> things down again

I think we should just assume caches are up to date. If we only cache
files that are distributed with Octave, and files that are installed
with the package manager, then I think it's fair to assume that these
caches are up to date. Really, if people change these functions, then I
doubt that it'll be the help text they're changing. The rest of the
users functions can just be searched file by file. And if the user
wants, then (s)he can just generate a cache manually.

> | I also have the following on my outstanding-issues-list:
> |
> |   * Perhaps move internal variables (eg 'makeinfo_program') to m-files?
>
> Yes, I think that would be good to do.

Okay, I'll make that change then. I was unsure because some of these
internal variables seems to be changeable through command line options
to Octave at startup (e.g. octave --info-file). Is that functionality we
can just drop?

> |   * how should 'private' directories be treated in Makefiles ?
>
> I'm not sure, but we do need to do something for this since there are
> other places where we could use private functions.

What I did was just to add 'private/myprivatefile.m' to the SRC list
along with 'myfile.m'. I think that works, but perhaps it's a bit ugly?

> |   * Fix XXX's in code (nothing major).
>
> Please use FIXME instead of XXX as people have complained in the past
> about patches/mail with XXX being flagged as spam.

Really? Perhaps it's a feature to flag my code as spam ;-)

Søren

[John W. Eaton]

On 29-Oct-2008, Søren Hauberg wrote:

| I think we should just assume caches are up to date. If we only cache
| files that are distributed with Octave, and files that are installed
| with the package manager, then I think it's fair to assume that these
| caches are up to date. Really, if people change these functions, then I
| doubt that it'll be the help text they're changing. The rest of the
| users functions can just be searched file by file. And if the user
| wants, then (s)he can just generate a cache manually.

OK.

| > | I also have the following on my outstanding-issues-list:
| > |
| > |   * Perhaps move internal variables (eg 'makeinfo_program') to m-files?
| >
| > Yes, I think that would be good to do.
|
| Okay, I'll make that change then. I was unsure because some of these
| internal variables seems to be changeable through command line options
| to Octave at startup (e.g. octave --info-file). Is that functionality we
| can just drop?

No, I think we should keep those options.  Instead of setting an
internal variable, the option handling code can feval the function to
reset the persistent variable in the function.  Does that make sense?
The action in octave_main may have to be deferred until it is safe to
call a function, but that should not be too difficult, I would guess.
Probably things like this should be done after the load_path is
initialized and just before executing the startup files.

| > |   * how should 'private' directories be treated in Makefiles ?
| >
| > I'm not sure, but we do need to do something for this since there are
| > other places where we could use private functions.
|
| What I did was just to add 'private/myprivatefile.m' to the SRC list
| along with 'myfile.m'. I think that works, but perhaps it's a bit ugly?

Did you try "make install"?  How are the private installation
directories created?

| > |   * Fix XXX's in code (nothing major).
| >
| > Please use FIXME instead of XXX as people have complained in the past
| > about patches/mail with XXX being flagged as spam.
|
| Really? Perhaps it's a feature to flag my code as spam ;-)

Aside from the complaints about spam filtering, I think it is good to
use a consistent style for these so they are easy to find and count.

jwe

[Søren Hauberg]

ons, 29 10 2008 kl. 17:26 -0400, skrev John W. Eaton:

> | > |   * Perhaps move internal variables (eg 'makeinfo_program') to m-files?
> | >
> | > Yes, I think that would be good to do.
> |
> | Okay, I'll make that change then. I was unsure because some of these
> | internal variables seems to be changeable through command line options
> | to Octave at startup (e.g. octave --info-file). Is that functionality we
> | can just drop?
>
> No, I think we should keep those options.  Instead of setting an
> internal variable, the option handling code can feval the function to
> reset the persistent variable in the function.  Does that make sense?
> The action in octave_main may have to be deferred until it is safe to
> call a function, but that should not be too difficult, I would guess.
> Probably things like this should be done after the load_path is
> initialized and just before executing the startup files.

Sounds a lot more complicated than the current situation. Is it really
worth it?

> | > |   * how should 'private' directories be treated in Makefiles ?
> | >
> | > I'm not sure, but we do need to do something for this since there are
> | > other places where we could use private functions.
> |
> | What I did was just to add 'private/myprivatefile.m' to the SRC list
> | along with 'myfile.m'. I think that works, but perhaps it's a bit ugly?
>
> Did you try "make install"?  How are the private installation
> directories created?

My bad -- sorry. The 'private' directly obviously isn't created, so no
this doesn't work.

> | > |   * Fix XXX's in code (nothing major).
> | >
> | > Please use FIXME instead of XXX as people have complained in the past
> | > about patches/mail with XXX being flagged as spam.
> |
> | Really? Perhaps it's a feature to flag my code as spam ;-)
>
> Aside from the complaints about spam filtering, I think it is good to
> use a consistent style for these so they are easy to find and count.

Yeah, I agree. I'll change it.

Søren

[John W. Eaton]

On 29-Oct-2008, Søren Hauberg wrote:

| ons, 29 10 2008 kl. 17:26 -0400, skrev John W. Eaton:
| > | > |   * Perhaps move internal variables (eg 'makeinfo_program') to m-files?
| > | >
| > | > Yes, I think that would be good to do.
| > |
| > | Okay, I'll make that change then. I was unsure because some of these
| > | internal variables seems to be changeable through command line options
| > | to Octave at startup (e.g. octave --info-file). Is that functionality we
| > | can just drop?
| >
| > No, I think we should keep those options.  Instead of setting an
| > internal variable, the option handling code can feval the function to
| > reset the persistent variable in the function.  Does that make sense?
| > The action in octave_main may have to be deferred until it is safe to
| > call a function, but that should not be too difficult, I would guess.
| > Probably things like this should be done after the load_path is
| > initialized and just before executing the startup files.
|
| Sounds a lot more complicated than the current situation. Is it really
| worth it?

Well, I'd rather not lose options that we've had for a long time if we
don't have to.  I can make the change if you would prefer.

| > | > |   * how should 'private' directories be treated in Makefiles ?
| > | >
| > | > I'm not sure, but we do need to do something for this since there are
| > | > other places where we could use private functions.
| > |
| > | What I did was just to add 'private/myprivatefile.m' to the SRC list
| > | along with 'myfile.m'. I think that works, but perhaps it's a bit ugly?
| >
| > Did you try "make install"?  How are the private installation
| > directories created?
|
| My bad -- sorry. The 'private' directly obviously isn't created, so no
| this doesn't work.

OK, I can try to look at this since we could probably use it for other
directories in the scripts hierarchy as well, but I can't promise when
I'll be able to get to it.

jwe

[Søren Hauberg]

ons, 29 10 2008 kl. 17:41 -0400, skrev John W. Eaton:
> | Sounds a lot more complicated than the current situation. Is it really
> | worth it?
>
> Well, I'd rather not lose options that we've had for a long time if we
> don't have to.  I can make the change if you would prefer.

Yes, I would prefer that. However, I'm not sure if it is a good change.
The current code is really simple as it just calls a macro. So, this
work only makes sense if we can remove all internal variables.

> | My bad -- sorry. The 'private' directly obviously isn't created, so no
> | this doesn't work.
>
> OK, I can try to look at this since we could probably use it for other
> directories in the scripts hierarchy as well, but I can't promise when
> I'll be able to get to it.

This isn't a high-priority thing, as the files in the 'private'
directory will still be copied during installation -- they just won't be
stored in a 'private' directory after installation.

Søren

[John W. Eaton]

On 29-Oct-2008, Søren Hauberg wrote:

| ons, 29 10 2008 kl. 17:41 -0400, skrev John W. Eaton:
| > | Sounds a lot more complicated than the current situation. Is it really
| > | worth it?
| >
| > Well, I'd rather not lose options that we've had for a long time if we
| > don't have to.  I can make the change if you would prefer.
|
| Yes, I would prefer that. However, I'm not sure if it is a good change.
| The current code is really simple as it just calls a macro. So, this
| work only makes sense if we can remove all internal variables.

I'm confused now.  What current code is that?

| This isn't a high-priority thing, as the files in the 'private'
| directory will still be copied during installation -- they just won't be
| stored in a 'private' directory after installation.

OK.

jwe

[Søren Hauberg]

ons, 29 10 2008 kl. 17:52 -0400, skrev John W. Eaton:
> | Yes, I would prefer that. However, I'm not sure if it is a good change.
> | The current code is really simple as it just calls a macro. So, this
> | work only makes sense if we can remove all internal variables.
>
> I'm confused now.  What current code is that?

Sorry about that. Currently the code to handle the 'info_file' variable
is handled with the following code (my changeset doesn't change this,
because I wasn't sure about the consequences)

DEFUN (info_file, args, nargout,
  "-*- texinfo -*-\n\
@deftypefn {Built-in Function} {@var{val} =} info_file ()\n\
@deftypefnx {Built-in Function} {@var{old_val} =} info_file
(@var{new_val})\n\
Query or set the internal variable that specifies the name of the\n\
Octave info file.  The default value is\n\
@code{\"@var{octave-home}/info/octave.info\"}, in\n\
which @var{octave-home} is the directory where all of Octave is
installed.\n\
@seealso{info_program, doc, help, makeinfo_program}\n\
@end deftypefn")
{
  return SET_NONEMPTY_INTERNAL_STRING_VARIABLE (info_file);
}

That seems quite simple to me. If you need to do a lot of work to move
this to an m-file, then I'm just not sure if it's worth the effort. But
hey, I haven't looked into the details, so I just might be missing the
obvious.

Søren

[John W. Eaton]

On 29-Oct-2008, Søren Hauberg wrote:

| ons, 29 10 2008 kl. 17:52 -0400, skrev John W. Eaton:
| > | Yes, I would prefer that. However, I'm not sure if it is a good change.
| > | The current code is really simple as it just calls a macro. So, this
| > | work only makes sense if we can remove all internal variables.
| >
| > I'm confused now.  What current code is that?
|
| Sorry about that. Currently the code to handle the 'info_file' variable
| is handled with the following code (my changeset doesn't change this,
| because I wasn't sure about the consequences)
|
| DEFUN (info_file, args, nargout,
|   "-*- texinfo -*-\n\
| @deftypefn {Built-in Function} {@var{val} =} info_file ()\n\
| @deftypefnx {Built-in Function} {@var{old_val} =} info_file
| (@var{new_val})\n\
| Query or set the internal variable that specifies the name of the\n\
| Octave info file.  The default value is\n\
| @code{\"@var{octave-home}/info/octave.info\"}, in\n\
| which @var{octave-home} is the directory where all of Octave is
| installed.\n\
| @seealso{info_program, doc, help, makeinfo_program}\n\
| @end deftypefn")
| {
|   return SET_NONEMPTY_INTERNAL_STRING_VARIABLE (info_file);
| }
|
| That seems quite simple to me. If you need to do a lot of work to move
| this to an m-file, then I'm just not sure if it's worth the effort. But
| hey, I haven't looked into the details, so I just might be missing the
| obvious.

OK, I see your point.  Let's leave these alone for now.

Thanks,

jwe

[Søren Hauberg]

Hi,
  I can't seem to figure out how to use Mercurial. Specifically, I can't
seem to figure out how to export changes that are relative to what I
checked out.

  hg export tip

exports my changes since last time I commited, but that'll only make it
possible to give you a changeset that is relative to my last changeset,
and I assume you'd prefer one changeset rather than a bunch that are all
relative to each other.

Any hints?
Søren

[Jaroslav Hajek-2]

On Fri, Oct 31, 2008 at 4:42 PM, Søren Hauberg <soren@...> wrote:

> Hi,
>  I can't seem to figure out how to use Mercurial. Specifically, I can't
> seem to figure out how to export changes that are relative to what I
> checked out.
>
>  hg export tip
>
> exports my changes since last time I commited, but that'll only make it
> possible to give you a changeset that is relative to my last changeset,
> and I assume you'd prefer one changeset rather than a bunch that are all
> relative to each other.
>
> Any hints?

One option is to use:

hg diff -r <revision you checked out>

but that doesn't give you a header. To avoid this, it's probably best
to use Mercurial queues. See the recent contrib.txi for an example.


> Søren
>
>



--
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz

[Søren Hauberg]

fre, 31 10 2008 kl. 17:04 +0100, skrev Jaroslav Hajek:
> One option is to use:
>
> hg diff -r <revision you checked out>

Okay, that seems simple. But how do I get that revision number?

Søren

[Søren Hauberg]

fre, 31 10 2008 kl. 18:06 +0100, skrev Søren Hauberg:
> fre, 31 10 2008 kl. 17:04 +0100, skrev Jaroslav Hajek:
> > One option is to use:
> >
> > hg diff -r <revision you checked out>
>
> Okay, that seems simple. But how do I get that revision number?

Never mind, I figured it out (it's the number before : in the changeset
number). Anyway, attached is a new diff, that does the same as my
previous changeset, plus

  * rename XXX to FIXME

  * add the documentation that Thorsten suggested

  * generate documentation cache during 'pkg install'

Søren