From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?B?7KGw7ISx67mI?= Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Add xwidget webkit support for macOS Cocoa Date: Sun, 26 May 2019 11:57:08 +0900 Message-ID: <679B30F5-4AB8-4F70-AEA1-A428538E928B@icloud.com> References: <20190525132727.94982-1-pcr910303@icloud.com> Mime-Version: 1.0 (1.0) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="34475"; mail-complaints-to="usenet@blaine.gmane.org" Cc: emacs-devel@gnu.org To: Paul Eggert Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun May 26 05:09:12 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.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hUjX4-0008ox-7C for ged-emacs-devel@m.gmane.org; Sun, 26 May 2019 05:09:10 +0200 Original-Received: from localhost ([127.0.0.1]:50112 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hUjX3-0002iV-7A for ged-emacs-devel@m.gmane.org; Sat, 25 May 2019 23:09:09 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:41028) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hUjWs-0002cy-HG for emacs-devel@gnu.org; Sat, 25 May 2019 23:08:59 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hUjLf-00054h-Ni for emacs-devel@gnu.org; Sat, 25 May 2019 22:57:24 -0400 Original-Received: from pv50p00im-zteg10011401.me.com ([17.58.6.41]:60635) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hUjLf-00054P-EP for emacs-devel@gnu.org; Sat, 25 May 2019 22:57:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=04042017; t=1558839441; bh=qDwFN50+9OAL1En0fDI039jyGdgfKuxMG+NVXdLqjXI=; h=Content-Type:Mime-Version:Subject:From:Date:Message-Id:To; b=g5C3H3NXNp3FmYUW5O0A/bAd/seqfeieB5K0Zqqzp2AvJYQ8oS5Hb4XDYR5NpKFFO aKKf/yUAfArqxDcLYjeMKbZ5PfFE3sS8zIaTTQwlHx/jueGt96Xg7tnyoBFk8UPZ/m Gt8XUUVGoVpoVwkxkyzdhxukWh1QsTbvzqIvN3Sa0qMGaL4r0fpKQ2WFXtQ4+S6sE0 m/AQ1ePFpWb8EsnVyfWHbfHCTjimGSC7glaWT6o9A1rlSbR7WGbIM8DOwhO4kzIOa3 63e7WGbelM3mQU6qBMd/aDgzLiWp8LKjYwP+eyYMhWKQCqck9hkJm/71upF69+bpE2 DZZj1ONKnjV4A== Original-Received: from [172.30.1.57] (unknown [211.192.227.198]) by pv50p00im-zteg10011401.me.com (Postfix) with ESMTPSA id 24BA9900859; Sun, 26 May 2019 02:57:21 +0000 (UTC) X-Mailer: iPhone Mail (16E227) In-Reply-To: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-05-26_01:, , signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=9 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1812120000 definitions=main-1905260019 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 17.58.6.41 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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:237003 Archived-At: 2019. 5. 26. =EC=98=A4=EC=A0=84 2:47, Paul Eggert =EC=9E= =91=EC=84=B1: > Thanks for doing all that work! Although I have no expertise in macOS I ha= ve a few minor comments about the patch from the perspective of someone who b= uilds and ports Emacs, and who wants to help navigate this patch through the= Emacs build bureaucracy. First, when I tried to use "git am" to apply the p= atch, I got these diagnostics about white-space problems that should be look= ed at: >=20 > Applying: Add xwidget webkit support for macOS Cocoa > .git/rebase-apply/patch:594: space before tab in indent. > for detail information about `NSApplicationDefinedMask'. --> > warning: 1 line adds whitespace errors. > nextstep/templates/Info.plist.in:682: space before tab in indent. > + for detail information about `NSApplicationDefinedMask'. --> Definitely a mistake, will fix as soon as I get home :-) > Next, when I tried to build by doing "./configure --enable-gcc-warnings --= with-xwidgets" on Fedora 30, I got the following diagnostics that should get= fixed: >=20 >=20 > xwidget.c:258:1: warning: no previous prototype for =E2=80=98store_xwidget= _event_string=E2=80=99 [-Wmissing-prototypes] > 258 | store_xwidget_event_string (struct xwidget *xw, const char *eventna= me, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > xwidget.c:272:1: warning: no previous prototype for =E2=80=98store_xwidget= _response_callback_event=E2=80=99 [-Wmissing-prototypes] > 272 | store_xwidget_response_callback_event (struct xwidget *xw, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > xwidget.c:292:1: warning: no previous prototype for =E2=80=98store_xwidget= _js_callback_event=E2=80=99 [-Wmissing-prototypes] > 292 | store_xwidget_js_callback_event (struct xwidget *xw, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Will add prototypes where appropriate... >> + WEBKIT_CFLAGS=3D"-D_REENTRANT -I/System/Library/Frameworks/WebKit.fr= amework/Headers" >=20 > Why is that -D_REENTRANT needed? I thought _REENTRANT was obsolete on macO= S. This was because this patch vas developed back from 2017 (by another person w= ho signed the FSF agreements and also expressed his intents to contribute bu= t didn=E2=80=99t...), and I was just maintaining it. Didn=E2=80=99t bother to fix that... will fix that too. >> - Does Emacs support Xwidgets (requires gtk3)? ${HAVE_XWIDGET= S} >> + Does Emacs support Xwidgets? ${HAVE_XWIDGET= S} >> + (requires gtk3 or macOS Cocoa) >=20 > Probably better to omit that second line; that part of the output is getti= ng too long anyway. Agreed. >> - syms_of_xwidget (); >> syms_of_xsettings (); >> #ifdef HAVE_X_SM >> syms_of_xsmfns (); >> @@ -1855,6 +1854,10 @@ Using an Emacs configured with --with-x-toolkit=3D= lucid does not have this problem >> #endif /* HAVE_W32NOTIFY */ >> #endif /* WINDOWSNT */ >> +#ifdef HAVE_XWIDGETS >> + syms_of_xwidget (); >> +#endif /* HAVE_XWIDGETS */ >> + >=20 > Why move the call to syms_of_xwidget? And why surround it with "#ifdef", s= ince syms_of_xwidget is a no-op if HAVE_XWIDGETS is not defined? It's better= to avoid #if when that's convenient. I vaguely remember that it once emitted an error when compiling... will try a= gain. >> +#include /* FIXME: Emacs error? message? instead of printf. *= / >=20 > Yes, we don't want printfs there. Can you give some candidates to use?=20 I=E2=80=99m actually not that proficient in programming elisp functions in C= ... :-( >> +#if defined (USE_GTK) >> #include >> #include >> +#elif defined (NS_IMPL_COCOA) >> +#include "nsxwidget.h" >> +#endif >=20 > Indent preprocessor directives consistently. No parens needed in "defined X= ". "#ifdef X" is easier to read than "#if defined X". Will fix. >> +#if defined (USE_GTK) >> #if WEBKIT_CHECK_VERSION (2, 21, 1) && GNUC_PREREQ (4, 2, 0) >> # pragma GCC diagnostic ignored "-Wdeprecated-declarations" >> #endif >> +#endif >=20 > Use just one "#if" rather than nested ones. That one was because the macro WEBKIT_CHECK_VERSION is only defined in GTK w= ebkit... Last time I looked, I remember being puzzled because C preprocessor= didn=E2=80=99t do proper short circuiting?=CC=8A=CC=88 I=E2=80=99ll try tha= t again. > I didn't look in detail at the .m or .el changes, but this is good enough f= or now. >=20