[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5] Enable xwidgets on macOS
From: |
조성빈 |
Subject: |
Re: [PATCH v5] Enable xwidgets on macOS |
Date: |
Sat, 27 Jul 2019 21:35:07 +0900 |
2019. 7. 27. 오후 7:41, Eli Zaretskii <address@hidden> 작성:
>> From: 조성빈 <address@hidden>
>> Date: Fri, 26 Jul 2019 03:51:11 +0900
>> Cc: address@hidden,
>> address@hidden
>> 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.
Ok, will do.
>> 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.
(I’m currently on my phone so I can’t really check but) I recall this part of
the checklist was specific to the NS port?
>> +(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.
Ok, will do.
>> +(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").
Ok, will do.
>> (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.
Ok, will do.
>> @@ -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?
Well, there wasn’t a particular problem, but it looked like that message was
more used for warnings or errors. Should I revert it?
>> +#elif defined NS_IMPL_COCOA
>> + nsxwidget_resize_view(xv, xw->width, xw->height);
> ^
> Space before the opening parenthesis is missing.
Ok, will fix.
>> +*** 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).
Ok, will fix.
>> +(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.
Ok, will do.
>> 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.
Ok, will fix.
>> +(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?
Well, it was meaning that the URL will be saved at the directory the user
specified in the prompt with the name FILE-NAME.
I’m not sure how I should reword it due to my poor English skills; can you
give me some suggestions?
- [PATCH v5] Enable xwidgets on macOS, (continued)
- [PATCH v5] Enable xwidgets on macOS, Sungbin Jo, 2019/07/19
- Re: [PATCH v5] Enable xwidgets on macOS, Alan Third, 2019/07/19
- Re: [PATCH v5] Enable xwidgets on macOS, Eli Zaretskii, 2019/07/20
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/23
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/25
- Re: [PATCH v5] Enable xwidgets on macOS, Eli Zaretskii, 2019/07/26
- Re: [PATCH v5] Enable xwidgets on macOS, Richard Stallman, 2019/07/26
- Re: [PATCH v5] Enable xwidgets on macOS, Eli Zaretskii, 2019/07/27
- Re: [PATCH v5] Enable xwidgets on macOS, Eli Zaretskii, 2019/07/27
- Re: [PATCH v5] Enable xwidgets on macOS,
조성빈 <=
- Re: [PATCH v5] Enable xwidgets on macOS, Eli Zaretskii, 2019/07/27
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/29
- Re: [PATCH v5] Enable xwidgets on macOS, Alan Third, 2019/07/29
- Re: [PATCH v5] Enable xwidgets on macOS, Stefan Monnier, 2019/07/29
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/30
- Re: [PATCH v5] Enable xwidgets on macOS, Stefan Monnier, 2019/07/30
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/31
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/30
- Re: [PATCH v5] Enable xwidgets on macOS, Juri Linkov, 2019/07/30
- Re: [PATCH v5] Enable xwidgets on macOS, 조성빈, 2019/07/31