bug#30068: 26.0; REGRESSION: no doc for `x-display-pixel-*'

[Glenn Morris]

Eli Zaretskii wrote:

>> Keep the real doc only in xfns.c, makes the others just say <skip>, or
>> <see:xfns.c>, and change Fsnarf_documentation to respect <skip>.
>
> Sounds fine.  Are there any downsides?

I can't think of any.

>> Related: I see Fsnarf_documentation already has skip_file, but it
>> doesn't handle lisp files. Maybe it could be taught about
>> preloaded-file-list, which would have avoided this particular problem.
>
> Not sure how: pc-win.el is preloaded (on some platforms).  Or maybe I
> misunderstand you.

If Fsnarf_documentation had skipped the pc-win definition on platforms
that did not load pc-win, the problem would have been confined to
platforms that do load pc-win. There's only one such: MS-DOS, where
no-one's interested in the doc of these functions, so it wouldn't matter.

> Btw, there's another, perhaps easier alternative: have only one
> definition of the function, with the single copy of the doc string,
> and then make it call the platform-dependent parts on the C level.

That would be (much) better, but I think it's (much) harder
(bug#4402 saw no interest in 8+ years).
If anyone ever does it, we won't need a workaround any more.

Here's the basic patch for my suggestion.
Turns out we can (and must) get rid of skip_files.
In addition, all the X/W32/NS "x-" doc-strings need to be combined so
that the X version describes all platforms. I thought this was already
done, but in one of the first cases I looked it, it wasn't. But this has
to be done anyway in the "real" solution, so the effort won't be wasted.


--- i/src/doc.c
+++ w/src/doc.c
@@ -535,7 +535,6 @@ it specifies the file name (without a directory) of the DOC 
file.
   EMACS_INT pos;
   Lisp_Object sym;
   char *p, *name;
-  bool skip_file = 0;
   ptrdiff_t count;
   char const *dirname;
   ptrdiff_t dirlen;
@@ -609,34 +608,24 @@ it specifies the file name (without a directory) of the 
DOC file.
        {
          end = strchr (p, '\n');
 
-          /* See if this is a file name, and if it is a file in build-files.  
*/
-          if (p[1] == 'S')
-            {
-              skip_file = 0;
-              if (end - p > 4 && end[-2] == '.'
-                  && (end[-1] == 'o' || end[-1] == 'c'))
-                {
-                  ptrdiff_t len = end - p - 2;
-                  char *fromfile = SAFE_ALLOCA (len + 1);
-                  memcpy (fromfile, &p[2], len);
-                  fromfile[len] = 0;
-                  if (fromfile[len-1] == 'c')
-                    fromfile[len-1] = 'o';
-
-                  skip_file = NILP (Fmember (build_string (fromfile),
-                                             Vbuild_files));
-                }
-            }
+         /* We used to skip files not in build_files, so that when a
+            function is defined several times in different files
+            (typically, once in xterm, once in w32term, ...), we only
+            pay attention to the one that matters.
+
+            But this means the doc has to be kept and updated in
+            multiple files. Nowadays we keep the doc only in eg xterm.
+            The (f)boundp checks below ensure we don't report
+            docs for eg w32-specific items on X.
+         */
 
          sym = oblookup (Vobarray, p + 2,
                          multibyte_chars_in_text ((unsigned char *) p + 2,
                                                   end - p - 2),
                          end - p - 2);
-         /* Check skip_file so that when a function is defined several
-            times in different files (typically, once in xterm, once in
-            w32term, ...), we only pay attention to the one that
-            matters.  */
-         if (! skip_file && SYMBOLP (sym))
+          /* Ignore docs that start with SKIP.  These mark
+             placeholders where the real doc is elsewhere.  */
+         if (SYMBOLP (sym))
            {
              /* Attach a docstring to a variable?  */
              if (p[1] == 'V')
@@ -644,8 +633,9 @@ it specifies the file name (without a directory) of the DOC 
file.
                  /* Install file-position as variable-documentation property
                     and make it negative for a user-variable
                     (doc starts with a `*').  */
-                  if (!NILP (Fboundp (sym))
+                  if ((!NILP (Fboundp (sym))
                       || !NILP (Fmemq (sym, delayed_init)))
+                      && strncmp (end, "\nSKIP", 5))
                     Fput (sym, Qvariable_documentation,
                           make_number ((pos + end + 1 - buf)
                                        * (end[1] == '*' ? -1 : 1)));
@@ -654,7 +644,7 @@ it specifies the file name (without a directory) of the DOC 
file.
              /* Attach a docstring to a function?  */
              else if (p[1] == 'F')
                 {
-                  if (!NILP (Ffboundp (sym)))
+                  if (!NILP (Ffboundp (sym)) && strncmp (end, "\nSKIP", 5))
                     store_function_docstring (sym, pos + end + 1 - buf);
                 }
              else if (p[1] == 'S')