[PATCH] Snarf-documentation was working by accident

[PATCH] Snarf-documentation was working by accident

[Daniel Colascione]

etc/DOC contains documentation for two kinds of symbol:

1) symbols in C
2) symbols in preloaded lisp

There's a test that makes sure a C file is used in a particular build of
Emacs before including its documentation. This test sets a flag if the
file is unused, and if this flag is set, we don't load any more
documentation until we see another file. Scenario 2 neither set nor
tested this flag, so it just used whatever value the flag happened to
have for the last C file; if the last C file happened to not be included
in this build of Emacs, the documentation for the Lisp file was never
loaded.

The effect was that documentation would mysteriously go missing. This
patch always includes documentation for Lisp files.

(Also, is all this lazy loading complexity really needed? etc/DOC is
only 2.3MB here.)

Load documentation from DOC
Index: emacs-23.2/src/doc.c
===================================================================
--- emacs-23.2.orig/src/doc.c
+++ emacs-23.2/src/doc.c
@@ -659,6 +659,12 @@ the same file name is found in the `doc-
 	      skip_file = NILP (Fmember (build_string (fromfile),
 					 Vbuild_files));
             }
+          else if(p[1] == 'S')
+            {
+              /* File is compiled lisp and its docstrings should
+                 always be included */
+              skip_file = 0;
+            }

 	  sym = oblookup (Vobarray, p + 2,
 			  multibyte_chars_in_text (p + 2, end - p - 2),

Re: [PATCH] Snarf-documentation was working by accident

[Jan Djärv]

Daniel Colascione skrev 2010-08-13 11.02:
> etc/DOC contains documentation for two kinds of symbol:
>
> 1) symbols in C
> 2) symbols in preloaded lisp
>
> There's a test that makes sure a C file is used in a particular build of
> Emacs before including its documentation. This test sets a flag if the
> file is unused, and if this flag is set, we don't load any more
> documentation until we see another file. Scenario 2 neither set nor
> tested this flag, so it just used whatever value the flag happened to
> have for the last C file; if the last C file happened to not be included
> in this build of Emacs, the documentation for the Lisp file was never
> loaded.
>
> The effect was that documentation would mysteriously go missing. This
> patch always includes documentation for Lisp files.
>
> (Also, is all this lazy loading complexity really needed? etc/DOC is
> only 2.3MB here.)
>

It is not for the size, it is the fact that some symbols are defined several 
times, like x-popup-dialog for X, W32 and NS.  We don't want the NS help 
string in an X build.

I fixed the issue in a different way.

Thanks for tracking this down.

	Jan D.

Re: [PATCH] Snarf-documentation was working by accident

[Daniel Colascione]

Thanks for taking a look at the problem.

On 8/13/10 3:35 AM, Jan Djärv wrote:
> It is not for the size, it is the fact that some symbols are defined
> several times, like x-popup-dialog for X, W32 and NS.  We don't want the
> NS help string in an X build.

That only applies to the C code though, right? There's also quite a bit
of logic surrounding delay-loading docstrings from *regular* compiled
elisp files. I just can't but wonder whether all that complexity is
still needed.

> I fixed the issue in a different way.

I don't think your fix does the right thing here.

The grammar of DOC is roughly:

DOC = FILE-ENTRY+
FILE-ENTRY = FILE-HEADER (VAR-ENTRY | FUNCTION-ENTRY)*
FILE-HEADER = "^_S" FILE-NAME "\n"
VAR-ENTRY = "^_V" SYMBOL-NAME "\n" DOCSTRING
FUNCTION-ENTRY = "^_F" SYMBOL-NAME "\n" DOCSTRING

Your patch resets skip_file each time we see a "^_", but skip_file is
supposed to apply to an entire file-entry. Consequently, if I'm reading
your patch correctly, skip_file is always 0 when we encounter a
VAR-ENTRY or FUNCTION-ENTRY, which will cause other problems.

(Also, the comment on line 643 is wrong and doesn't talk about the
FILE-HEADER case.)

Re: [PATCH] Snarf-documentation was working by accident

[Jan Djärv]

Daniel Colascione skrev 2010-08-13 18.31:

> That only applies to the C code though, right? There's also quite a bit
> of logic surrounding delay-loading docstrings from *regular* compiled
> elisp files. I just can't but wonder whether all that complexity is
> still needed.

As long as it works nobody seems motivated to change it.

>
> I don't think your fix does the right thing here.
>
> The grammar of DOC is roughly:
>
> DOC = FILE-ENTRY+
> FILE-ENTRY = FILE-HEADER (VAR-ENTRY | FUNCTION-ENTRY)*
> FILE-HEADER = "^_S" FILE-NAME "\n"
> VAR-ENTRY = "^_V" SYMBOL-NAME "\n" DOCSTRING
> FUNCTION-ENTRY = "^_F" SYMBOL-NAME "\n" DOCSTRING
>
> Your patch resets skip_file each time we see a "^_", but skip_file is
> supposed to apply to an entire file-entry. Consequently, if I'm reading
> your patch correctly, skip_file is always 0 when we encounter a
> VAR-ENTRY or FUNCTION-ENTRY, which will cause other problems.
>
> (Also, the comment on line 643 is wrong and doesn't talk about the
> FILE-HEADER case.)

You are quite right, I was too fast.  I updated the code.

	Jan D.