From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Alex Gramiak Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Renaming non-X x_* identifiers Date: Sat, 13 Apr 2019 21:35:01 -0600 Message-ID: <875zrhtg2i.fsf@gmail.com> References: <87wokp4okn.fsf@gmail.com> <0f4be9a6-6e09-f55d-9f58-2a15aef264cd@cs.ucla.edu> <837ecpplw8.fsf@gnu.org> <871s2w510a.fsf@gmail.com> <922F9B91-2E9E-45F6-BB96-66CAE5E9FB81@gnu.org> <87k1goqpnn.fsf@gmail.com> <83imw8nspc.fsf@gnu.org> <87ftrcqg5j.fsf@gmail.com> <83bm20nm62.fsf@gnu.org> <87d0men4jx.fsf@gmail.com> <83o95sisk7.fsf@gnu.org> <87mulcnui4.fsf@gmail.com> <83bm1si7lf.fsf@gnu.org> <87ef6ont03.fsf@gmail.com> <83a7hci44l.fsf@gnu.org> <87a7hcndtc.fsf@gmail.com> <831s2nhza8.fsf@gnu.org> <87d0lpvq6n.fsf_-_@gmail.com> <83r2a5keo7.fsf@gnu.org> <87pnppu4ox.fsf@gmail.com> <83muktk9xb.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="173483"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) Cc: alan@idiocy.org, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Apr 14 05:35:48 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 1hFVvm-000iwW-0b for ged-emacs-devel@m.gmane.org; Sun, 14 Apr 2019 05:35:46 +0200 Original-Received: from localhost ([127.0.0.1]:58825 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hFVvk-0004EF-OC for ged-emacs-devel@m.gmane.org; Sat, 13 Apr 2019 23:35:44 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:51012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hFVvd-0004CT-JV for emacs-devel@gnu.org; Sat, 13 Apr 2019 23:35:38 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hFVv8-00046S-ED for emacs-devel@gnu.org; Sat, 13 Apr 2019 23:35:07 -0400 Original-Received: from mail-pf1-x442.google.com ([2607:f8b0:4864:20::442]:37530) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hFVv8-000460-6L; Sat, 13 Apr 2019 23:35:06 -0400 Original-Received: by mail-pf1-x442.google.com with SMTP id 8so6969667pfr.4; Sat, 13 Apr 2019 20:35:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=1WXWm2OQFS60fpI4rGLpmplITFP21gHR4jagKwIF0lI=; b=cchjzrls+KxTnChG2AQEYvmaDPkkp9m8xzdFzZuuBkgFVgHXR5WQV095S08gDOhR6L 0qzIhZYz3srt1GX9rPH7I9/VsoDfAC6VmBsyb7L41P1zCHcxL3yuRcjEE2QR7Vk663CO phjnXNbIaHnlWouXdgApmUcjay4Gjy1YhrFqom9YHzlPZbivPIFruBk8YV6JybTSP+nE /nBId/3JcTgoor7GSd7DysDueExfwo3bApC0QN0/9wKcWMEGPCjeZhacadW0O3nx7Iej e8Yx2e4RNSV/QggGarHt/05FNxx8C2opd7mPhldQjhfZNTUgFIeM8ogj+Kd+P4cVIPdA 4WLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=1WXWm2OQFS60fpI4rGLpmplITFP21gHR4jagKwIF0lI=; b=sEFImWFxGSF8HyVM1t4Qnmq0dWg3geRfcgYZfasoaHIsufVD+LgBDZlQ6Tw0wmU+Dg 7FS1CRZrH5k6aFwA+XofR5BffxWVpvhOhvgVzJC4RyhoKDC960eKPyg6JSxAxLWC64LY pjrEKpuHZSws70HmiaggJRKgUFOxSI9OeCdGzNcteJoN28tidNwuvRE8xHkFnZ58I1xZ 5CehhxS6tVqjOruC11WWY2ymsB41d683l+ZG1gYNcHLKkHuJLP883kisj7GlgHGgSdwS PzO3pyveAp79cXEzIIokA6hk/Dyx7YnXUytEo7XQfQD5D1Crz99i6g92ALiYQHn+FCod QNqA== X-Gm-Message-State: APjAAAW85UO3/MMlqu1jFYCIHbOacl/KB1ZUnv64QrjZuNNALtruBrZW DfE8LkXaawjQo4DwfIwWZNrvZPEk X-Google-Smtp-Source: APXvYqy/83sj/F1b0ltOxSvqXUjUGiYKc0A10UsQvbyW1OMge6n6Mt/sZFacb7qRqSO51H0JDY0gRg== X-Received: by 2002:aa7:8e04:: with SMTP id c4mr31692772pfr.48.1555212904389; Sat, 13 Apr 2019 20:35:04 -0700 (PDT) Original-Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302]) by smtp.gmail.com with ESMTPSA id t9sm11125040pgp.66.2019.04.13.20.35.03 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 13 Apr 2019 20:35:03 -0700 (PDT) In-Reply-To: <83muktk9xb.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 13 Apr 2019 22:00:16 +0300") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::442 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:235419 Archived-At: Eli Zaretskii writes: > Not sure I understand what you mean by "policy". Rebasing or not in > general is up to you, but maybe you are asking about something more > specific. I mean specifically about rebasing during the review process. Though it turns out that the Savannah git server doesn't allow this, so I just deleted and repushed the entire branch instead. > If every window-system is required to provide these hooks, then I > think it will be enough to test only those which also have > implementations on TTY frames. Okay. How about wrapping the required hooks in termhooks.h in #ifdef HAVE_WINDOW_SYSTEM so that terminal-only builds that erroneously have these hooks in their scope issue a compiler error? >> >> store_frame_param (f, prop, val); >> >> >> >> - param_index = Fget (prop, Qx_frame_parameter); >> >> + param_index = Fget (prop, Qframe_parameter_pos); >> > >> > The x-frame-parameter property is visible from Lisp, no? You are here >> > replacing it with a different symbol, which is a backward-incompatible >> > change. >> >> While it is visible from Lisp, I don't see why anyone would change it >> considering that AFAIU it's used as an internal value in frame.c. >> frame.c sets it and uses the value of the property to call the >> appropriate element in frame_parm_table, which Lisp-code should not rely >> on. >> >> Then again, apparently cedet/semantic/util-modes.el accesses this >> property, but that could be changed. > > Anything that gets put into frame-parameters can have some Lisp out > there using it. So I think we have 2 alternatives: > > 1) leave those symbols alone > 2) declare them obsolete, but meanwhile put both the new and the old > symbols into frame-parameters > > The above assumes that if a Lisp program does something with one of > these parameters, that will have no effect, i.e. that these parameters > are one-way communications from the Emacs internals to Lisp, as far as > Lisp programs are concerned. If the communications are two-way, then > I don't see how we can change these names; do you have any ideas? AFAIU it's technically possible that someone could use `put' to set a new value, but that's tantamount to changing the internal definition of the frame parameter setter to another frame parameter setter, so I don't think such a use case should really be considered. I don't have any other ideas, but 2) doesn't sound terrible as long as it would be removed some day. Though I don't feel strongly about the symbols here. > It's okay to do that in a followup, but please do that soon. I don't > want to risk leaving an unfinished job in the sources. I just pushed commits implementing this hook and the other one discussed. > I think using image_* for all the functions in image.c would be > better. Okay, done. >> >> - (f, Qx_set_fullscreen, 0, 0, list2 (old_value, fullscreen)); >> >> + (f, Qgui_set_fullscreen, 0, 0, list2 (old_value, fullscreen)); >> > >> > This is also visible from Lisp, right? So renaming the symbol would >> > be an incompatible change. >> >> I believe frame_size_history_add only uses the symbols as a >> visual/debugging aid, so I don't believe this is, meaningfully, an >> incompatible change. > > Are they in frame-parameters? If so, they are visible. No, but they are used in the variable frame-size-history which is used by frame--size-history. I'm not sure if this is significant enough to warrant leaving it alone. >> >> diff --git a/src/menu.h b/src/menu.h >> >> index 0321c27454..4412948224 100644 >> >> --- a/src/menu.h >> >> +++ b/src/menu.h >> >> @@ -47,14 +47,17 @@ extern widget_value *digest_single_submenu (int, int, bool); >> >> #if defined (HAVE_X_WINDOWS) || defined (MSDOS) >> >> extern Lisp_Object x_menu_show (struct frame *, int, int, int, >> >> Lisp_Object, const char **); >> >> +extern void x_activate_menubar (struct frame *); >> >> #endif >> >> #ifdef HAVE_NTGUI >> >> extern Lisp_Object w32_menu_show (struct frame *, int, int, int, >> >> Lisp_Object, const char **); >> >> +extern void w32_activate_menubar (struct frame *); >> >> #endif >> >> #ifdef HAVE_NS >> >> extern Lisp_Object ns_menu_show (struct frame *, int, int, int, >> >> Lisp_Object, const char **); >> >> +extern void ns_activate_menubar (struct frame *); >> > >> > Since you introduced activate_menubar_hook, why do we need to declare >> > prototypes for its implementation on menu.h, which is a >> > system-independent header? >> >> The implementations are defined in the *menu.c files, but are added as >> terminal hooks in the *term.c files. > > I'm not sure I understand the answer. I didn't ask about the > implementations, I asked about the prototypes. Since these are hooks, > their names are not visible outside the corresponding *term.c file, > right? Then why do we need the prototypes of w32_activate_menubar, > ns_activate_menubar, etc. in menu.h? The declarations are to make the names visible to *term.c. *menu.c contains the actual definitions, so *term.c needs declarations to set the hook. It's why *_menu_show are there as well, even though they are terminal hooks.