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

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

[Drew Adams]

In Emacs 25.3.1, `C-h f x-display-pixel-width':

 x-display-pixel-width is a built-in function in 'C source code'.

 (x-display-pixel-width &optional DISPLAY)

 Return the width in pixels of DISPLAY.
 The optional argument DISPLAY specifies which display to ask about.
 DISPLAY should be either a frame or a display name (a string).
 If omitted or nil, that stands for the selected frame's display.

 On "multi-monitor" setups this refers to the pixel width for all
 physical monitors associated with DISPLAY.  To get information for
 each physical monitor, use 'display-monitor-attributes-list'.

In Emacs 26 pretest:

 x-display-pixel-width is a built-in function in 'C source code'.

 (x-display-pixel-width &optional FRAME)

 Not documented.

I find nothing in NEWS about this.



In GNU Emacs 26.0.90 (build 3, x86_64-w64-mingw32)
 of 2017-10-13
Repository revision: 906224eba147bdfc0514090064e8e8f53160f1d4
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

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

[Eli Zaretskii]

> Date: Wed, 10 Jan 2018 10:11:41 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> In Emacs 26 pretest:
> 
>  x-display-pixel-width is a built-in function in 'C source code'.
> 
>  (x-display-pixel-width &optional FRAME)
> 
>  Not documented.

Thanks, fixed.

> I find nothing in NEWS about this.

We don't yet document bugs in NEWS, let alone unknown bugs.

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

[Glenn Morris]

The doc for these x- functions is now stored in quadruplicate
(src/xfns.c, src/w32fns.c, src/nsfns.m, lisp/term/pc-win.el).

Is there a better way?

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

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: eliz@gnu.org
> Date: Wed, 10 Jan 2018 14:41:42 -0500
> 
> 
> The doc for these x- functions is now stored in quadruplicate
> (src/xfns.c, src/w32fns.c, src/nsfns.m, lisp/term/pc-win.el).
> 
> Is there a better way?

I hope there is, and I'm all for seeking one.

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

[Glenn Morris]

Eli Zaretskii wrote:

>> The doc for these x- functions is now stored in quadruplicate
>> (src/xfns.c, src/w32fns.c, src/nsfns.m, lisp/term/pc-win.el).
>> 
>> Is there a better way?
>
> I hope there is, and I'm all for seeking one.

How about:

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>.

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.

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

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 30068@debbugs.gnu.org
> Date: Thu, 11 Jan 2018 14:26:02 -0500
> 
> 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?

> 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.

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

[Eli Zaretskii]

> Date: Thu, 11 Jan 2018 22:19:18 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 30068@debbugs.gnu.org
> 
> > From: Glenn Morris <rgm@gnu.org>
> > Cc: 30068@debbugs.gnu.org
> > Date: Thu, 11 Jan 2018 14:26:02 -0500
> > 
> > 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?

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.

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

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

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 30068@debbugs.gnu.org
> Date: Fri, 12 Jan 2018 19:16:09 -0500
> 
> >> 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.

So you want to special-case pc-win.el?  I thought this was a more
general proposal.

> 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'm not sure I understand: I thought we made all the doc strings of
such functions identical some time ago, to avoid this issue.  Which
doc strings are not identical between different implementations?

> +          /* Ignore docs that start with SKIP.  These mark
> +             placeholders where the real doc is elsewhere.  */

This must be in the ELisp manual, of course, and perhaps also in NEWS.

Thanks.

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

[Glenn Morris]

Eli Zaretskii wrote:

> So you want to special-case pc-win.el?

No.

>  I thought this was a more general proposal.

Yes.

>> 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'm not sure I understand: I thought we made all the doc strings of
> such functions identical some time ago, to avoid this issue.  Which
> doc strings are not identical between different implementations?

The first one I happened to look at was x-server-max-request-size.
Cf nsfns.m and xfns.c.

>> +          /* Ignore docs that start with SKIP.  These mark
>> +             placeholders where the real doc is elsewhere.  */
>
> This must be in the ELisp manual, of course, and perhaps also in NEWS.

I don't see why an internal implementation detail that will be
self-documenting ("SKIP: real doc in xfns.c") needs to be in the
manual and NEWS, but if you say so.


Is this change then approved?

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

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 30068@debbugs.gnu.org
> Date: Sat, 20 Jan 2018 13:14:06 -0500
> 
> >> +          /* Ignore docs that start with SKIP.  These mark
> >> +             placeholders where the real doc is elsewhere.  */
> >
> > This must be in the ELisp manual, of course, and perhaps also in NEWS.
> 
> I don't see why an internal implementation detail that will be
> self-documenting ("SKIP: real doc in xfns.c") needs to be in the
> manual and NEWS, but if you say so.

We need this in the manual so that people who write such doc strings
will know to use this facility in the future.

> Is this change then approved?

I'm okay with it, yes.

Thanks.

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > The doc for these x- functions is now stored in quadruplicate
  > (src/xfns.c, src/w32fns.c, src/nsfns.m, lisp/term/pc-win.el).

  > Is there a better way?

We could surely implement one, if it that is less work
that maintaining these doc strings in 4 places.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.