From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH v5] Enable xwidgets on macOS Date: Sat, 27 Jul 2019 13:41:01 +0300 Message-ID: <831rybn4jm.fsf@gnu.org> References: <20190718192321.65684-1-pcr910303@icloud.com> <20190719041654.88561-1-pcr910303@icloud.com> <83pnm5t8ui.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="139638"; mail-complaints-to="usenet@blaine.gmane.org" Cc: alan@idiocy.org, emacs-devel@gnu.org To: =?utf-8?B?7KGw7ISx67mI?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jul 27 12:41:20 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hrK8e-000aEA-9u for ged-emacs-devel@m.gmane.org; Sat, 27 Jul 2019 12:41:20 +0200 Original-Received: from localhost ([::1]:44892 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hrK8d-0003Fl-3b for ged-emacs-devel@m.gmane.org; Sat, 27 Jul 2019 06:41:19 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:54288) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hrK8V-0003Fe-EK for emacs-devel@gnu.org; Sat, 27 Jul 2019 06:41:12 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:38533) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hrK8U-00070a-P0; Sat, 27 Jul 2019 06:41:10 -0400 Original-Received: from [176.228.60.248] (port=1670 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hrK8U-0001fW-14; Sat, 27 Jul 2019 06:41:10 -0400 In-reply-to: (message from =?utf-8?B?7KGw7ISx67mI?= on Fri, 26 Jul 2019 03:51:11 +0900) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:238945 Archived-At: > From: 조성빈 > Date: Fri, 26 Jul 2019 03:51:11 +0900 > Cc: alan@idiocy.org, > emacs-devel@gnu.org > > Hello, I have prepared three patches to apply on master. Thanks. Alan, could you please review the NS specific parts? > diff --git a/etc/NEWS b/etc/NEWS > index 5313270411..52b49bab75 100644 > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -2440,6 +2440,18 @@ was able to 'set' modifiers without the knowledge of the user. > ** On NS multicolor font display is enabled again since it is also > implemented in Emacs on free operating systems via Cairo drawing. > > +** On macOS, Xwidget is now supported. > +If Emacs was built with xwidget support, you can access the embedded > +webkit browser with 'M-x xwidget-webkit-browse-url'. Viewing two > +instances of xwidget webkit is not supported. > + > +*** New functions for xwidget-webkit mode > +'xwidget-webkit-clone-and-split-below', > +'xwidget-webkit-clone-and-split-right'. > + > +*** New variable 'xwidget-webkit-enable-plugins'. The last two entries are not specific to macOS, are they? If so, they should be in the general sections of NEWS. > diff --git a/etc/TODO b/etc/TODO > index a065763933..bda1cf8f9e 100644 > --- a/etc/TODO > +++ b/etc/TODO > @@ -640,15 +640,6 @@ from the emacsclient process. > > This sections contains features found in other official Emacs ports. > > -**** Support for xwidgets > - > -Emacs 25 has support for xwidgets, a system to include operating > -system components into an Emacs buffer. The components range from > -simple buttons to webkit (effectively, a web browser). > - > -Currently, xwidgets works only for the gtk+ framework but it is > -designed to be compatible with multiple Emacs ports. The MS-Windows and non-GTK builds of Emacs on X still don't support xwidgets, so I think this entry should not be deleted in its entirety, it should still say that some configurations don't support xwidgets. > +(defun xwidget-webkit-clone-and-split-below () > + "Clone current URL into a new widget place in new window below. > +Get the URL of current session, then browse to the URL > +in `split-window-below' with a new xwidget webkit session." The second sentence should be reworded: Get the URL of current session, then browse that URL in another window after splitting the selected window horizontally. > +(defun xwidget-webkit-clone-and-split-right () > + "Clone current URL into a new widget place in new window right. > +Get the URL of current session, then browse to the URL > +in `split-window-right' with a new xwidget webkit session." Same here (but use "vertically" instead of "horizontally"). > (defvar bookmark-make-record-function) > +(when (memq window-system '(mac ns)) > + (defvar xwidget-webkit-enable-plugins nil > + "Enable plugins for xwidget webkit. > +If non-nil, plugins are enabled. Otherwise, disabled.")) It is better to have this defvar unconditionally, and tell in the doc string that it only has effect on macOS. > @@ -228,13 +247,14 @@ offscreen_damage_event (GtkWidget *widget, GdkEvent *event, > if (GTK_IS_WIDGET (xv_widget)) > gtk_widget_queue_draw (GTK_WIDGET (xv_widget)); > else > - printf ("Warning, offscreen_damage_event received invalid xv pointer:%p\n", > - xv_widget); > + message ("Warning, offscreen_damage_event received invalid xv pointer:%p\n", > + xv_widget); Why replace printf by message? > +#elif defined NS_IMPL_COCOA > + nsxwidget_resize_view(xv, xw->width, xw->height); ^ Space before the opening parenthesis is missing. > +*** Functions 'xwidget-webkit-scroll-up', 'xwidget-webkit-scroll-down' > +now supports scrolling arbitrary pixel values. It now treats the ^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ "support" (plural) "by arbitrary pixel values". "They now treat" (plural). > +(defun xwidget-webkit-scroll-up-line (&optional n) > + "Scroll webkit up by N lines. > +The height of line is calculated with `window-font-height'. > +Stop if the bottom edge of the page is reached. > +If N is omitted or nil, scroll up by one line." > + (interactive "p") > + (xwidget-webkit-scroll-up (* n (window-font-height)))) > + > +(defun xwidget-webkit-scroll-down-line (&optional n) > + "Scroll webkit down by N lines. > +The height of line is calculated with `window-font-height'. > +Stop if the top edge of the page is reached. > +If N is omitted or nil, scroll down by one line." > + (interactive "p") > + (xwidget-webkit-scroll-down (* n (window-font-height)))) > + > +(defun xwidget-webkit-scroll-forward (&optional n) > + "Scroll webkit horizontally by N chars. > +The width of char is calculated with `window-font-width'. > +If N is ommited or nil, scroll forwards by one char." > + (interactive "p") > (xwidget-webkit-execute-script > (xwidget-webkit-current-session) > - "window.scrollBy(50, 0);")) > - > -(defun xwidget-webkit-scroll-backward () > - "Scroll webkit backwards." > - (interactive) > + (format "window.scrollBy(%d, 0);" > + (* n (window-font-width))))) > + > +(defun xwidget-webkit-scroll-backward (&optional n) > + "Scroll webkit back by N chars. > +The width of char is calculated with `window-font-width'. > +If N is ommited or nil, scroll backwards by one char." > + (interactive "p") These commands should say in their doc strings that interactively N is the prefix numeric argument. > diff --git a/etc/NEWS b/etc/NEWS > index f9a42f73be..c7f980f212 100644 > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -2469,6 +2469,10 @@ instances of xwidget webkit is not supported. > > *** New variable 'xwidget-webkit-enable-plugins'. > > +** On macOS, downloading files from xwidget-webkit is supported. > + > +*** New variable 'xwidget-webkit-download-dir'. The macOS-specific part of this should be in the non-free OS part of NEWS. > +(defun xwidget-webkit-save-as-file (url mime-type file-name) > + "For XWIDGET webkit, save URL of MIME-TYPE to location specified by user. > +FILE-NAME combined with `xwidget-webkit-download-dir' is the default file name > +of the prompt when reading. When the file name the user specified is a > +directory, URL is saved at the specified directory as FILE-NAME." The last sentence is unclear: what will be the directory where the URL will be saved, and what will be the name of the saved file in that directory?