* bug#45620: 28.0.50; Child frames should have their own border width and colour @ 2021-01-03 13:24 Alexander Miller 2021-01-03 16:15 ` martin rudalics ` (8 more replies) 0 siblings, 9 replies; 27+ messages in thread From: Alexander Miller @ 2021-01-03 13:24 UTC (permalink / raw) To: 45620 This is an idea set off by this discussion: https://github.com/Alexander-Miller/treemacs/issues/242#issuecomment-753296634 Basically the problem is that child frames need a distinct border colour to be properly visible (and just using a different background colour does not look nearly as good). That can be achieved by customising the `internal-border' face, however that face controls the appearance of the border of all frames, and as I have learned there is a non-trivial amount of users who by default use a large internal border as a margin for their frames. And for these users the `internal-border' should have the same colour as the default background. So there is a conflict between wanting a distinct border colour for child frames and using the default background for normal frames. Packages can work around that individually, for example posframe accepts an `internal-border-color' argument that overrides the `internal-border' face frame-locally. But that still means that every package using child frames needs its own user option for the child frames' border colour when such matters clearly belong under the domain of the user's theme. I think that since child frames serve sufficiently distinct use cases than normal frames it makes sense for them to have their own border appearance controls. In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.23, cairo version 1.17.3) of 2020-12-20 built on am-laptop Repository revision: ab985f41db5fdaeada513d28a065332fd8838cf4 Repository branch: makepkg Windowing system distributor 'The X.Org Foundation', version 11.0.12008000 System Description: Manjaro Linux Configured using: 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games --with-sound=alsa --with-modules --without-gconf --without-gsettings --with-nativecomp --with-x-toolkit=gtk3 --without-xaw3d --without-m17n-flt --with-cairo --without-compress-install 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -g -fuse-ld=gold' CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now' Configured features: XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GLIB NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES NATIVE_COMP THREADS LIBSYSTEMD JSON PDUMPER LCMS2 Important settings: value of $LC_MONETARY: de_DE.UTF-8 value of $LC_NUMERIC: de_DE.UTF-8 value of $LC_TIME: de_DE.UTF-8 value of $LANG: en_GB.utf8 locale-coding-system: utf-8 Major mode: Fundamental Minor modes in effect: helm-mode: t helm--remap-mouse-mode: t async-bytecomp-package-mode: t projectile-mode: t company-prescient-mode: t global-company-mode: t company-mode: t global-evil-vimish-fold-mode: t show-smartparens-global-mode: t show-smartparens-mode: t smartparens-global-mode: t smartparens-mode: t org-super-agenda-mode: t treemacs-icons-dired-mode: t treemacs-filewatch-mode: t treemacs-follow-mode: t treemacs-git-mode: deferred treemacs-fringe-indicator-mode: t gcmh-mode: t winner-mode: t framey-mode: t purpose-mode: t shackle-mode: t winum-mode: t eyebrowse-mode: t global-ligature-mode: t ligature-mode: t global-subword-mode: t subword-mode: t global-evil-surround-mode: t evil-surround-mode: t evil-lion-mode: t evil-goggles-mode: t shell-dirtrack-mode: t evil-mode: t evil-local-mode: t tooltip-mode: t global-eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Load-path shadows: ~/Documents/git/cfrs/cfrs hides /home/am/.emacs.d/straight/build/cfrs/cfrs /usr/share/emacs/28.0.50/lisp/emacs-lisp/let-alist hides /home/am/.emacs.d/straight/build/let-alist/let-alist ~/Documents/git/treemacs/src/elisp/treemacs-interface hides /home/am/.emacs.d/straight/build/treemacs/treemacs-interface ~/Documents/git/treemacs/src/elisp/treemacs-rendering hides /home/am/.emacs.d/straight/build/treemacs/treemacs-rendering ~/Documents/git/treemacs/src/elisp/treemacs-hydras hides /home/am/.emacs.d/straight/build/treemacs/treemacs-hydras ~/Documents/git/treemacs/src/elisp/treemacs-dom hides /home/am/.emacs.d/straight/build/treemacs/treemacs-dom ~/Documents/git/treemacs/src/elisp/treemacs-tags hides /home/am/.emacs.d/straight/build/treemacs/treemacs-tags ~/Documents/git/treemacs/src/elisp/treemacs hides /home/am/.emacs.d/straight/build/treemacs/treemacs ~/Documents/git/treemacs/src/elisp/treemacs-workspaces hides /home/am/.emacs.d/straight/build/treemacs/treemacs-workspaces ~/Documents/git/treemacs/src/elisp/treemacs-customization hides /home/am/.emacs.d/straight/build/treemacs/treemacs-customization ~/Documents/git/treemacs/src/elisp/treemacs-faces hides /home/am/.emacs.d/straight/build/treemacs/treemacs-faces ~/Documents/git/treemacs/src/elisp/treemacs-themes hides /home/am/.emacs.d/straight/build/treemacs/treemacs-themes ~/Documents/git/treemacs/src/elisp/treemacs-mouse-interface hides /home/am/.emacs.d/straight/build/treemacs/treemacs-mouse-interface ~/Documents/git/treemacs/src/elisp/treemacs-mode hides /home/am/.emacs.d/straight/build/treemacs/treemacs-mode ~/Documents/git/treemacs/src/elisp/treemacs-icons hides /home/am/.emacs.d/straight/build/treemacs/treemacs-icons ~/Documents/git/treemacs/src/elisp/treemacs-compatibility hides /home/am/.emacs.d/straight/build/treemacs/treemacs-compatibility ~/Documents/git/treemacs/src/elisp/treemacs-follow-mode hides /home/am/.emacs.d/straight/build/treemacs/treemacs-follow-mode ~/Documents/git/treemacs/src/elisp/treemacs-visuals hides /home/am/.emacs.d/straight/build/treemacs/treemacs-visuals ~/Documents/git/treemacs/src/elisp/treemacs-core-utils hides /home/am/.emacs.d/straight/build/treemacs/treemacs-core-utils ~/Documents/git/treemacs/src/elisp/treemacs-extensions hides /home/am/.emacs.d/straight/build/treemacs/treemacs-extensions ~/Documents/git/treemacs/src/elisp/treemacs-filewatch-mode hides /home/am/.emacs.d/straight/build/treemacs/treemacs-filewatch-mode ~/Documents/git/treemacs/src/elisp/treemacs-persistence hides /home/am/.emacs.d/straight/build/treemacs/treemacs-persistence ~/Documents/git/treemacs/src/elisp/treemacs-async hides /home/am/.emacs.d/straight/build/treemacs/treemacs-async ~/Documents/git/treemacs/src/elisp/treemacs-bookmarks hides /home/am/.emacs.d/straight/build/treemacs/treemacs-bookmarks ~/Documents/git/treemacs/src/elisp/treemacs-tag-follow-mode hides /home/am/.emacs.d/straight/build/treemacs/treemacs-tag-follow-mode ~/Documents/git/treemacs/src/elisp/treemacs-logging hides /home/am/.emacs.d/straight/build/treemacs/treemacs-logging ~/Documents/git/treemacs/src/elisp/treemacs-header-line hides /home/am/.emacs.d/straight/build/treemacs/treemacs-header-line ~/Documents/git/treemacs/src/elisp/treemacs-fringe-indicator hides /home/am/.emacs.d/straight/build/treemacs/treemacs-fringe-indicator ~/Documents/git/treemacs/src/elisp/treemacs-diagnostics hides /home/am/.emacs.d/straight/build/treemacs/treemacs-diagnostics ~/Documents/git/treemacs/src/elisp/treemacs-macros hides /home/am/.emacs.d/straight/build/treemacs/treemacs-macros ~/Documents/git/treemacs/src/elisp/treemacs-scope hides /home/am/.emacs.d/straight/build/treemacs/treemacs-scope Features: (shadow ispell rainbow-delimiters rainbow-mode xterm-color display-line-numbers hl-todo prose-complete emacsbug face-remap helm-command helm-elisp helm-eval edebug backtrace helm-info info sort gnus-cite smiley shr-color color mm-archive mail-extr gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum shr kinsoku svg dom gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range gnus-win gnus nnheader wid-edit pp view helm-mode helm-projectile helm-files docker-tramp tramp-cache tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat parse-time iso8601 ls-lisp helm-tags helm-buffers helm-occur helm-grep helm-regexp helm-utils helm-locate helm-help helm-types framey-helm helm-config helm async-bytecomp helm-global-bindings helm-source helm-multi-match helm-lib async treemacs-projectile projectile grep compile ibuf-ext ibuffer ibuffer-loaddefs mu4e-alert time alert log4e notifications dbus company-keywords company-dabbrev-code company-dabbrev company-yasnippet company-files company-capf company-prescient prescient company server evil-vimish-fold vimish-fold smartparens-config smartparens-org smartparens-text paren smartparens xml gntp org-mu4e mu4e desktop frameset mu4e-org german-holidays org-super-agenda ts org-habit org-element avl-tree generator org-agenda org-refile org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete org-list org-faces org-entities noutline outline org-version ob-emacs-lisp ob-core ob-eval org-table ol org-keys org-compat org-macs org-loaddefs find-func mu4e-main mu4e-view cal-menu calendar cal-loaddefs browse-url url url-proxy url-privacy url-expand url-methods url-history url-cookie url-domsuf url-util url-parse url-vars mailcap mu4e-headers mu4e-compose mu4e-context mu4e-draft mu4e-actions ido rfc2368 smtpmail sendmail mu4e-mark mu4e-message flow-fill mu4e-proc mu4e-utils doc-view jka-compr mu4e-lists mule-util mu4e-vars message rmc puny treemacs-icons-dired treemacs-evil treemacs treemacs-header-line treemacs-compatibility treemacs-mode treemacs-interface treemacs-extensions treemacs-mouse-interface treemacs-tags imenu xref project treemacs-persistence treemacs-filewatch-mode filenotify treemacs-follow-mode treemacs-rendering treemacs-async treemacs-workspaces treemacs-dom treemacs-visuals treemacs-fringe-indicator treemacs-scope treemacs-faces treemacs-icons treemacs-themes treemacs-core-utils pfuture hl-line treemacs-logging treemacs-customization treemacs-macros dired+ image-dired image-mode exif image-file image-converter dired-x dired-aux dired dired-loaddefs rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail rmail-loaddefs auth-source password-cache json map text-property-search time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader mu4e-meta gcmh doom-modeline doom-modeline-segments doom-modeline-env doom-modeline-core shrink-path f all-the-icons all-the-icons-faces data-material data-weathericons data-octicons data-fileicons data-faicons data-alltheicons memoize winner framey inline ht s window-purpose window-purpose-fixes window-purpose-prefix-overload window-purpose-switch let-alist window-purpose-layout window-purpose-core window-purpose-configuration eieio-compat eieio window-purpose-utils shackle trace winum eyebrowse format-spec ligature morning-star-theme cap-words superword subword evil-surround evil-lion evil-goggles pulse evil evil-keybindings evil-integration evil-maps evil-commands reveal evil-jumps evil-command-window evil-types evil-search evil-ex shell pcomplete comint ansi-color evil-macros evil-repeat evil-states evil-core comp comp-cstr warnings subr-x cl-extra help-mode seq byte-opt bytecomp byte-compile cconv advice evil-common windmove thingatpt rect evil-digraphs evil-vars ring edmacro kmacro cl-seq dash yequake-autoloads yasnippet-autoloads yaml-mode-autoloads xwidgets-reuse-autoloads xterm-color-autoloads wttrin-autoloads writeroom-mode-autoloads with-editor-autoloads winum-autoloads window-purpose-autoloads eieio-core cl-macs eieio-loaddefs vterm-autoloads visual-fill-column-autoloads vimish-fold-autoloads ts-autoloads tridactylrc-mode-autoloads treepy-autoloads treemacs-autoloads transient-posframe-autoloads transient-autoloads toml-mode-autoloads toc-org-autoloads tablist-autoloads straight-autoloads spinner-autoloads smartparens-autoloads shrink-path-autoloads shackle-autoloads s-autoloads rust-mode-autoloads restart-emacs-autoloads rainbow-mode-autoloads rainbow-delimiters-autoloads projectile-autoloads pretty-hydra-autoloads prescient-autoloads posframe-autoloads pos-tip-autoloads popup-autoloads pkg-info-autoloads pfuture-autoloads perspective-autoloads persp-mode-autoloads peep-dired-autoloads pdf-tools-autoloads pcre2el-autoloads package-lint-autoloads org-superstar-autoloads org-super-agenda-autoloads org-autoloads multi-compile-autoloads mu4e-views-autoloads mu4e-alert-autoloads morning-star-autoloads memoize-autoloads markdown-mode-autoloads magit-todos-autoloads magit-autoloads macrostep-autoloads lv-autoloads lsp-ui-autoloads lsp-treemacs-autoloads lsp-mode-autoloads log4e-autoloads link-hint-autoloads ligature-autoloads let-alist-autoloads ledger-mode-autoloads json-snatcher-autoloads json-reformat-autoloads json-mode-autoloads rx imenu-list-autoloads i3wm-config-mode-autoloads hydra-autoloads ht-autoloads hl-todo-autoloads helpful-autoloads helm-projectile-autoloads helm-org-autoloads helm-easymenu easymenu helm-core-autoloads helm-ag-autoloads helm-autoloads goto-chg-autoloads gntp-autoloads git-modes-autoloads git-gutter-fringe-autoloads git-gutter-autoloads git-commit-autoloads ghub-autoloads german-holidays-autoloads gcmh-autoloads frog-menu-autoloads fringe-helper-autoloads framey-autoloads frame-local-autoloads forge-autoloads flyspell-correct-autoloads flycheck-pos-tip-autoloads flycheck-autoloads fish-mode-autoloads fill-column-indicator-autoloads f-autoloads eyebrowse-autoloads expand-region-autoloads evil-vimish-fold-autoloads evil-surround-autoloads evil-numbers-autoloads evil-nerd-commenter-autoloads evil-magit-autoloads evil-lion-autoloads evil-ledger-autoloads evil-goggles-autoloads evil-exchange-autoloads evil-collection-autoloads evil-autoloads eros-autoloads epl-autoloads epkg-autoloads emacsql-sqlite-autoloads emacsql-autoloads elisp-refs-autoloads elfeed-org-autoloads elfeed-autoloads doom-modeline-autoloads doct-autoloads dockerfile-mode-autoloads docker-tramp-autoloads docker-compose-mode-autoloads docker-autoloads dired+-autoloads dash-functional-autoloads dash-autoloads ctrlf-autoloads easy-mmode company-shell-autoloads company-quickhelp-autoloads company-prescient-autoloads company-box-autoloads company-autoloads closql-autoloads cfrs-autoloads buttercup-autoloads avy-autoloads async-autoloads anzu-autoloads annalist-autoloads all-the-icons-autoloads alert-autoloads ace-window-autoloads cl-loaddefs cl-lib gv iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button loaddefs faces cus-face pcase macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote threads dbusbind inotify lcms2 dynamic-setting font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty make-network-process nativecomp emacs) Memory information: ((conses 16 668840 513173) (symbols 48 43798 0) (strings 32 152762 59495) (string-bytes 1 5475171) (vectors 16 63689) (vector-slots 8 1094531 320683) (floats 8 926 1405) (intervals 56 6773 8846) (buffers 984 21)) ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller @ 2021-01-03 16:15 ` martin rudalics 2021-01-04 13:38 ` Alexander Miller ` (7 subsequent siblings) 8 siblings, 0 replies; 27+ messages in thread From: martin rudalics @ 2021-01-03 16:15 UTC (permalink / raw) To: Alexander Miller, 45620 > This is an idea set off by this discussion: > https://github.com/Alexander-Miller/treemacs/issues/242#issuecomment-753296634 > > Basically the problem is that child frames need a distinct border colour > to be properly visible (and just using a different background colour does > not look nearly as good). > > That can be achieved by customising the `internal-border' face, however > that face controls the appearance of the border of all frames, and as I > have learned there is a non-trivial amount of users who by default use a > large internal border as a margin for their frames. And for these users > the `internal-border' should have the same colour as the default > background. Isn't the situation even worse than how you describe it here? When I customize 'internal-border' face, that affects all frames, including those for which I have set it already via 'set-face-background'. Which means that whatever a package does to set that face for a specific (child) frame, that setting is undone by a later customization. IIUC the discussion you refer to above arrived at the same conclusion. > So there is a conflict between wanting a distinct border colour for > child frames and using the default background for normal frames. If I'm not mistaken we use that face for our tooltip frames too which means one more conflict. > Packages can work around that individually, for example posframe accepts > an `internal-border-color' argument that overrides the `internal-border' > face frame-locally. But that still means that every package using child > frames needs its own user option for the child frames' border colour > when such matters clearly belong under the domain of the user's theme. "clearly" is clearly too strong here. Ultimately, the package must have the choice and its choice should prevail (it currently doesn't). > I think that since child frames serve sufficiently distinct use cases > than normal frames it makes sense for them to have their own border > appearance controls. So what should we do? Provide a separate 'child-frame-internal-border' face and then probably also a 'tooltip-internal-border-face'? Customizing such a face would still override individual frame settings. What we need is probably a strategy to avoid setting the background for those frames that have their internal border already set. But then we should do that for all faces running through 'set-face-attribute'. martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller 2021-01-03 16:15 ` martin rudalics @ 2021-01-04 13:38 ` Alexander Miller 2021-01-04 16:22 ` martin rudalics 2021-01-04 17:48 ` Alexander Miller ` (6 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-04 13:38 UTC (permalink / raw) To: rudalics; +Cc: 45620 > Isn't the situation even worse than how you describe it here? When I > customize 'internal-border' face, that affects all frames, including > those for which I have set it already via 'set-face-background'. Which > means that whatever a package does to set that face for a specific > (child) frame, that setting is undone by a later customization. IIUC > the discussion you refer to above arrived at the same conclusion. What customisations are you referring to? I cannot think of any scenario, other than changing and reloading your theme, that could change the settings of already present child frames. > If I'm not mistaken we use that face for our tooltip frames too which > means one more conflict. Partially. On my system the internal-border colour and width only applied to the top and left sides of the tooltip frame. > "clearly" is clearly too strong here. Ultimately, the package must have > the choice and its choice should prevail (it currently doesn't). Packages do have the choice, they just have to make sure to override the frame-local faces whenever they show something. The problem, as I see it, is that they *have to* create a face to override the local internal-border, instead of just having the option to do it, because themes cannot offer a general setting, like an easily visible dark border on a bright foreground, because they run into the frame margin colour conflict like modus did in the linked issue. So if a package doesn't overwrite the local internal-border your default look-and-feel is a badly visible same-colour-on-same-colour popup. > So what should we do? Provide a separate 'child-frame-internal-border' > face and then probably also a 'tooltip-internal-border-face'? That would be perfectly good enough for themes like modus, yes. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-04 13:38 ` Alexander Miller @ 2021-01-04 16:22 ` martin rudalics 0 siblings, 0 replies; 27+ messages in thread From: martin rudalics @ 2021-01-04 16:22 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620 > What customisations are you referring to? I cannot think of any > scenario, other than changing and reloading your theme, that could > change the settings of already present child frames. Here I'm using a child frame for popping up the minibuffer with the customization (defcustom pop-up-mini-internal-border "blue" "Background of internal border of 'pop-up-mini-frame'." ... and the form (set-face-background 'internal-border pop-up-mini-internal-border pop-up-mini-frame) to put it into practice when setting up such a frame. This works fine until I do M-x customize-face RET internal-border RET. As soon as I apply the value I customized there, my minibuffer child frame gets that border instead of the one specified via 'pop-up-mini-internal-border' above. > > If I'm not mistaken we use that face for our tooltip frames too which > > means one more conflict. > > Partially. On my system the internal-border colour and width only > applied to the top and left sides of the tooltip frame. Yes. I think there's a bug somewhere but the only time I tried to locate it, I decided that someone else intentionally does it that way. > Packages do have the choice, they just have to make sure to override the > frame-local faces whenever they show something. The problem, as I see > it, is that they *have to* create a face to override the local > internal-border, instead of just having the option to do it, because > themes cannot offer a general setting, like an easily visible dark > border on a bright foreground, because they run into the frame margin > colour conflict like modus did in the linked issue. So if a package > doesn't overwrite the local internal-border your default look-and-feel > is a badly visible same-colour-on-same-colour popup. Agreed. On a historical note, I had to add the entire internal border entertainment to child frames because neither gtk nor X bother to supply the normal window manager border for them (Windows does) and so you can neither make them stand apart easily nor can you resize them with the mouse. Implementing the latter from within Emacs was even very entertaining. I intentionally left the border color alone because I'm convinced that customizing it is a bad idea as sketched in the above scenario and packages should always try their own way to specify it. > > So what should we do? Provide a separate 'child-frame-internal-border' > > face and then probably also a 'tooltip-internal-border-face'? > > That would be perfectly good enough for themes like modus, yes. Can you propose a patch? Look at where we use INTERNAL_BORDER_FACE_ID to set up a face (x_clear_under_internal_border in xterm.c is an example), check f for child-frameness there and use the new face if the frame is a child frame. But be aware: Users like me will then have to separately set the background for the child frame face so you might get incompatibility complaints ... martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller 2021-01-03 16:15 ` martin rudalics 2021-01-04 13:38 ` Alexander Miller @ 2021-01-04 17:48 ` Alexander Miller 2021-01-04 18:54 ` martin rudalics 2021-01-05 12:50 ` Alexander Miller ` (5 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-04 17:48 UTC (permalink / raw) To: rudalics; +Cc: 45620 > As soon as I apply the value I customized there, my minibuffer child > frame gets that border instead of the one specified via > 'pop-up-mini-internal-border' above. Personally I would think this behaviour is mildly inconvenient at worst, but hardly a real problem. It's just a new global default overriding local customisations. And that is only if I manage to run into the situation in the first place - I am not usually in the habit of customising frame border faces, and I don't think many other people are, either. So from my perspective I don't really see the danger (or annoyance) in this behaviour. > Can you propose a patch? I can *try*. I am absolutely not a C programmer, but as long as the task is limited to a monkey see, monkey do situation for handling a new face I should be able to hammer something useful together. In fact my first attempt seems to compile and behave as expected, so I have a few questions on how to proceed: - I need to repeatedly use a pattern that looks like this: int is_child_frame = FRAME_PARENT_FRAME(f) != NULL; int border_face_id = is_child_frame ? CHILD_FRAME_BORDER_FACE_ID : INTERNAL_BORDER_FACE_ID; int face_id = !NILP (Vface_remapping_alist) ? lookup_basic_face (NULL, f, border_face_id) : border_face_id; and I would like to put it into a single function accessible from anywhere. Is that the right call, and if yes, where would be the right place to put it? - Currently the actual width of the border is still controlled by the `internal-border-width` parameter for both frame types. Should I try to do something about that as well? If yes, what's my entry point? - I think I'll need to sign the FSF copyright assignment, unless the limit is higher than the 15 lines I am remembering. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-04 17:48 ` Alexander Miller @ 2021-01-04 18:54 ` martin rudalics 0 siblings, 0 replies; 27+ messages in thread From: martin rudalics @ 2021-01-04 18:54 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, Feng Shu > > Can you propose a patch? > > I can *try*. I am absolutely not a C programmer, but as long as the task > is limited to a monkey see, monkey do situation for handling a new face > I should be able to hammer something useful together. That's one way to become a C programmer ... > In fact my first attempt seems to compile and behave as expected, so I > have a few questions on how to proceed: > > - I need to repeatedly use a pattern that looks like this: > > int is_child_frame = FRAME_PARENT_FRAME(f) != NULL; In that case you should make is_child_frame a bool and not an int. But it's much simpler to just test for FRAME_PARENT_FRAME (f) wherever you see a need for is_child_frame like in > int border_face_id = is_child_frame int border_face_id = FRAME_PARENT_FRAME (f) > ? CHILD_FRAME_BORDER_FACE_ID > : INTERNAL_BORDER_FACE_ID; > int face_id = !NILP (Vface_remapping_alist) > ? lookup_basic_face (NULL, f, border_face_id) > : border_face_id; > > and I would like to put it into a single function accessible from > anywhere. Is that the right call, and if yes, where would be the right > place to put it? This is the first time I see the internal border face getting remapped. I wasn't aware that nsterm.c does that and I'm not sure whether we should add something similar to xterm.c and w32term.c. In nsterm.c I would not write an extra function but instead of what we have now use int face_id = (FRAME_PARENT_FRAME (f) ? (!NILP (Vface_remapping_alist) ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) : CHILD_FRAME_BORDER_FACE_ID) : (!NILP (Vface_remapping_alist) ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) : INTERNAL_BORDER_FACE_ID)); and in lookup_basic_face in xfaces.c you'd then have to add case CHILD_FRAME_BORDER_FACE_ID: name = Qchild_frame_border; break; It's augmenting forms like that last one that get the most obscure bugs, so don't despair. > - Currently the actual width of the border is still controlled by the > `internal-border-width` parameter for both frame types. Should I try to > do something about that as well? If yes, what's my entry point? Add a 'child-frame-border-width' parameter. But in this case I would propose to proceed as follows: - If for a frame the 'child-frame-border-width' was explicitly set, use it. - If it was not set, use the 'internal-border-width' parameter. Note that people have already set up their own child frame creation routines and we should try to not punish them too much. And please try to discuss this on the list you cited earlier and also with the posframe.el designer. Mister Feng Shu you've inevitably become a participant of this thread now. > - I think I'll need to sign the FSF copyright assignment, unless the > limit is higher than the 15 lines I am remembering. I think so too. martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller ` (2 preceding siblings ...) 2021-01-04 17:48 ` Alexander Miller @ 2021-01-05 12:50 ` Alexander Miller 2021-01-05 15:33 ` martin rudalics 2021-01-06 16:32 ` Alexander Miller ` (4 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-05 12:50 UTC (permalink / raw) To: rudalics; +Cc: 45620, tumashu [-- Attachment #1: Type: text/plain, Size: 2195 bytes --] >>> Can you propose a patch? >> >> I can *try*. I am absolutely not a C programmer, but as long as the task >> is limited to a monkey see, monkey do situation for handling a new face >> I should be able to hammer something useful together. > > That's one way to become a C programmer ... To be honest I've don't really like C as a language, and learning it would not do anything professionally for me either. So I'm jumping into this strictly for the sake of improving Emacs. >> and I would like to put it into a single function accessible from >> anywhere. Is that the right call, and if yes, where would be the right >> place to put it? > > This is the first time I see the internal border face getting remapped. > I wasn't aware that nsterm.c does that and I'm not sure whether we > should add something similar to xterm.c and w32term.c. In nsterm.c I > would not write an extra function but instead of what we have now use > > int face_id = > (FRAME_PARENT_FRAME (f) > ? (!NILP (Vface_remapping_alist) > ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) > : CHILD_FRAME_BORDER_FACE_ID) > : (!NILP (Vface_remapping_alist) > ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) > : INTERNAL_BORDER_FACE_ID)); x, ns and w32 all used the same code, so I changed them all to look like this now. >> - Currently the actual width of the border is still controlled by the >> `internal-border-width` parameter for both frame types. Should I try to >> do something about that as well? If yes, what's my entry point? > > Add a 'child-frame-border-width' parameter. But in this case I would > propose to proceed as follows: > > - If for a frame the 'child-frame-border-width' was explicitly set, use > it. > > - If it was not set, use the 'internal-border-width' parameter. That's done now too, at least for X. I'll attach patches of my first working drafts for both changes. Let me know if I'm on the right path so far. >> - I think I'll need to sign the FSF copyright assignment, unless the >> limit is higher than the 15 lines I am remembering. > > I think so too. > Ok, what do I do? [-- Attachment #2: 0001-WIP-child-frame-border.patch --] [-- Type: text/x-patch, Size: 6405 bytes --] From 50e3d31abf4bd12e24f82bb2f8e39ec1c3c073e0 Mon Sep 17 00:00:00 2001 From: Alexander Miller <alexanderm@web.de> Date: Tue, 5 Jan 2021 11:33:31 +0100 Subject: [PATCH 1/2] WIP child-frame-border --- lisp/faces.el | 11 ++++++++++- src/dispextern.h | 1 + src/nsterm.m | 10 +++++++--- src/w32fns.c | 10 +++++++--- src/xfaces.c | 3 +++ src/xterm.c | 20 ++++++++++++++------ 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/lisp/faces.el b/lisp/faces.el index 4e98338432..639dbeb09a 100644 --- a/lisp/faces.el +++ b/lisp/faces.el @@ -2683,11 +2683,20 @@ window-divider-last-pixel (defface internal-border '((t nil)) - "Basic face for the internal border." + "Basic face for the internal border. +For the internal border of child frames see `child-frame-border'." :version "26.1" :group 'frames :group 'basic-faces) +(defface child-frame-border + '((t nil)) + "Basic face for the internal border of child frames. +For the internal border of non-child frames see `internal-border'." + :version "28.1" + :group 'frames + :group 'basic-faces) + (defface minibuffer-prompt '((((background dark)) :foreground "cyan") ;; Don't use blue because many users of the MS-DOS port customize diff --git a/src/dispextern.h b/src/dispextern.h index 3ad98b8344..f4e872644d 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -1826,6 +1826,7 @@ #define FACE_UNIBYTE_P(FACE) ((FACE)->charset < 0) WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID, WINDOW_DIVIDER_LAST_PIXEL_FACE_ID, INTERNAL_BORDER_FACE_ID, + CHILD_FRAME_BORDER_FACE_ID, TAB_BAR_FACE_ID, TAB_LINE_FACE_ID, BASIC_FACE_ID_SENTINEL diff --git a/src/nsterm.m b/src/nsterm.m index 2defb9e2ee..76f67d2531 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -3029,9 +3029,13 @@ so some key presses (TAB) are swallowed by the system. */ NSRectEdge edge[] = {NSMinXEdge, NSMinYEdge, NSMaxXEdge, NSMaxYEdge}; int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); if (!face) diff --git a/src/w32fns.c b/src/w32fns.c index c1e18ff7fa..061886f0bc 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -1519,9 +1519,13 @@ w32_clear_under_internal_border (struct frame *f) int width = FRAME_PIXEL_WIDTH (f); int height = FRAME_PIXEL_HEIGHT (f); int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); block_input (); diff --git a/src/xfaces.c b/src/xfaces.c index b3b19a9cb2..5080120957 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -4913,6 +4913,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id) case WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID: name = Qwindow_divider_first_pixel; break; case WINDOW_DIVIDER_LAST_PIXEL_FACE_ID: name = Qwindow_divider_last_pixel; break; case INTERNAL_BORDER_FACE_ID: name = Qinternal_border; break; + case CHILD_FRAME_BORDER_FACE_ID: name = Qchild_frame_border; break; default: emacs_abort (); /* the caller is supposed to pass us a basic face id */ @@ -5619,6 +5620,7 @@ realize_basic_faces (struct frame *f) realize_named_face (f, Qwindow_divider_last_pixel, WINDOW_DIVIDER_LAST_PIXEL_FACE_ID); realize_named_face (f, Qinternal_border, INTERNAL_BORDER_FACE_ID); + realize_named_face (f, Qchild_frame_border, CHILD_FRAME_BORDER_FACE_ID); realize_named_face (f, Qtab_bar, TAB_BAR_FACE_ID); realize_named_face (f, Qtab_line, TAB_LINE_FACE_ID); @@ -6967,6 +6969,7 @@ syms_of_xfaces (void) DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel"); DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel"); DEFSYM (Qinternal_border, "internal-border"); + DEFSYM (Qchild_frame_border, "child-frame-border"); /* TTY color-related functions (defined in tty-colors.el). */ DEFSYM (Qtty_color_desc, "tty-color-desc"); diff --git a/src/xterm.c b/src/xterm.c index 0a86738cc2..92dcd95043 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -1293,9 +1293,13 @@ x_clear_under_internal_border (struct frame *f) int height = FRAME_PIXEL_HEIGHT (f); int margin = FRAME_TOP_MARGIN_HEIGHT (f); int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); block_input (); @@ -1360,9 +1364,13 @@ x_after_update_window_line (struct window *w, struct glyph_row *desired_row) { int y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, desired_row->y)); int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); block_input (); -- 2.29.2 [-- Attachment #3: 0002-WIP-child-frame-border-width.patch --] [-- Type: text/x-patch, Size: 5925 bytes --] From b176909995eea600d99354be2127469f472ce4bb Mon Sep 17 00:00:00 2001 From: Alexander Miller <alexanderm@web.de> Date: Tue, 5 Jan 2021 13:31:09 +0100 Subject: [PATCH 2/2] WIP child-frame-border-width --- src/frame.c | 4 ++++ src/frame.h | 14 +++++++++++++- src/xfns.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/frame.c b/src/frame.c index 45ee96e962..d5040dfae6 100644 --- a/src/frame.c +++ b/src/frame.c @@ -3744,6 +3744,7 @@ DEFUN ("set-frame-window-state-change", Fset_frame_window_state_change, {"foreground-color", -1}, {"icon-name", SYMBOL_INDEX (Qicon_name)}, {"icon-type", SYMBOL_INDEX (Qicon_type)}, + {"child-frame-border-width", SYMBOL_INDEX (Qchild_frame_border_width)}, {"internal-border-width", SYMBOL_INDEX (Qinternal_border_width)}, {"right-divider-width", SYMBOL_INDEX (Qright_divider_width)}, {"bottom-divider-width", SYMBOL_INDEX (Qbottom_divider_width)}, @@ -4287,6 +4288,8 @@ gui_report_frame_params (struct frame *f, Lisp_Object *alistptr) store_in_alist (alistptr, Qborder_width, make_fixnum (f->border_width)); + store_in_alist (alistptr, Qchild_frame_border_width, + make_fixnum (CHILD_FRAME_BORDER_WIDTH (f))); store_in_alist (alistptr, Qinternal_border_width, make_fixnum (FRAME_INTERNAL_BORDER_WIDTH (f))); store_in_alist (alistptr, Qright_divider_width, @@ -5984,6 +5987,7 @@ syms_of_frame (void) DEFSYM (Qhorizontal_scroll_bars, "horizontal-scroll-bars"); DEFSYM (Qicon_name, "icon-name"); DEFSYM (Qicon_type, "icon-type"); + DEFSYM (Qchild_frame_border_width, "child-frame-border-width"); DEFSYM (Qinternal_border_width, "internal-border-width"); DEFSYM (Qleft_fringe, "left-fringe"); DEFSYM (Qline_spacing, "line-spacing"); diff --git a/src/frame.h b/src/frame.h index 8cf41dc004..7d816f2516 100644 --- a/src/frame.h +++ b/src/frame.h @@ -534,6 +534,10 @@ #define EMACS_FRAME_H /* Border width of the frame window as known by the (X) window system. */ int border_width; + /* Width of child frames' internal border. Acts exactly like + internal_border_width below and will fall back on it when not set.*/ + int child_frame_border_width; + /* Width of the internal border. This is a line of background color just inside the window's border. When the frame is selected, a highlighting is displayed inside the internal border. */ @@ -1432,11 +1436,19 @@ FRAME_TOTAL_FRINGE_WIDTH (struct frame *f) return FRAME_LEFT_FRINGE_WIDTH (f) + FRAME_RIGHT_FRINGE_WIDTH (f); } +INLINE int +CHILD_FRAME_BORDER_WIDTH (struct frame *f) +{ + return frame_dimension (f->child_frame_border_width); +} + /* Pixel-width of internal border lines. */ INLINE int FRAME_INTERNAL_BORDER_WIDTH (struct frame *f) { - return frame_dimension (f->internal_border_width); + return f->child_frame_border_width + ? CHILD_FRAME_BORDER_WIDTH(f) + : frame_dimension (f->internal_border_width); } /* Pixel-size of window divider lines. */ diff --git a/src/xfns.c b/src/xfns.c index 9ab537ca8d..663146d7f2 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -1800,6 +1800,28 @@ x_change_tool_bar_height (struct frame *f, int height) #endif /* USE_GTK */ } +static void +x_set_child_frame_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) +{ + int border = check_int_nonnegative (arg); + + if (border != CHILD_FRAME_BORDER_WIDTH (f)) + { + f->child_frame_border_width = border; + +#ifdef USE_X_TOOLKIT + if (FRAME_X_OUTPUT (f)->edit_widget) + widget_store_internal_border (FRAME_X_OUTPUT (f)->edit_widget); +#endif + + if (FRAME_X_WINDOW (f)) + { + adjust_frame_size (f, -1, -1, 3, false, Qchild_frame_border_width); + x_clear_under_internal_border (f); + } + } + +} static void x_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) @@ -3897,14 +3919,37 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame, parms = Fcons (Fcons (Qinternal_border_width, value), parms); } + /* Same for child frames. */ + if (NILP (Fassq (Qchild_frame_border_width, parms))) + { + Lisp_Object value; + + value = gui_display_get_arg (dpyinfo, parms, Qchild_frame_border_width, + "internalBorder", "internalBorder", + RES_TYPE_NUMBER); + if (! EQ (value, Qunbound)) + parms = Fcons (Fcons (Qchild_frame_border_width, value), + parms); + + } + + gui_default_parameter (f, parms, Qchild_frame_border_width, +#ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets. */ + make_fixnum (0), +#else + make_fixnum (1), +#endif + "childFrameBorderWidth", "childFrameBorderWidth", + RES_TYPE_NUMBER); gui_default_parameter (f, parms, Qinternal_border_width, #ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets. */ - make_fixnum (0), + make_fixnum (0), #else - make_fixnum (1), + make_fixnum (1), #endif - "internalBorderWidth", "internalBorderWidth", - RES_TYPE_NUMBER); + "internalBorderWidth", "internalBorderWidth", + RES_TYPE_NUMBER); + gui_default_parameter (f, parms, Qright_divider_width, make_fixnum (0), NULL, NULL, RES_TYPE_NUMBER); gui_default_parameter (f, parms, Qbottom_divider_width, make_fixnum (0), @@ -7762,6 +7807,7 @@ DEFUN ("x-gtk-debug", Fx_gtk_debug, Sx_gtk_debug, 1, 1, 0, x_set_foreground_color, x_set_icon_name, x_set_icon_type, + x_set_child_frame_border_width, x_set_internal_border_width, gui_set_right_divider_width, gui_set_bottom_divider_width, -- 2.29.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-05 12:50 ` Alexander Miller @ 2021-01-05 15:33 ` martin rudalics 2021-01-05 15:34 ` martin rudalics 2021-01-05 16:26 ` Eli Zaretskii 0 siblings, 2 replies; 27+ messages in thread From: martin rudalics @ 2021-01-05 15:33 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > To be honest I've don't really like C as a language, and learning it > would not do anything professionally for me either. So I'm jumping into > this strictly for the sake of improving Emacs. Then please jump into this as often as you want. > x, ns and w32 all used the same code, so I changed them all to look like > this now. OK. I think a main reason that we usually do not factor out code in these is that only one of them gets compiled in anyway - so we just save an extra call. > >> - Currently the actual width of the border is still controlled by the > >> `internal-border-width` parameter for both frame types. Should I try to > >> do something about that as well? If yes, what's my entry point? > > > > Add a 'child-frame-border-width' parameter. But in this case I would > > propose to proceed as follows: > > > > - If for a frame the 'child-frame-border-width' was explicitly set, use > > it. > > > > - If it was not set, use the 'internal-border-width' parameter. > > That's done now too, at least for X. I'll attach patches of my first > working drafts for both changes. Let me know if I'm on the right path so > far. You're already at the end of it. Please document the changes in the Elisp manual and announce them as (probably incompatible) changes in NEWS. > >> - I think I'll need to sign the FSF copyright assignment, unless the > >> limit is higher than the 15 lines I am remembering. > > > > I think so too. > > > > Ok, what do I do? Eli, can you send Arthur the forms please? Thanks, martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-05 15:33 ` martin rudalics @ 2021-01-05 15:34 ` martin rudalics 2021-01-06 11:32 ` Arthur Miller 2021-01-05 16:26 ` Eli Zaretskii 1 sibling, 1 reply; 27+ messages in thread From: martin rudalics @ 2021-01-05 15:34 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > Eli, can you send Arthur the forms please? ^^^^^ Alexander, obviously ... martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-05 15:34 ` martin rudalics @ 2021-01-06 11:32 ` Arthur Miller 2021-01-06 13:36 ` martin rudalics 0 siblings, 1 reply; 27+ messages in thread From: Arthur Miller @ 2021-01-06 11:32 UTC (permalink / raw) To: martin rudalics; +Cc: 45620, Alexander Miller, tumashu martin rudalics <rudalics@gmx.at> writes: >> Eli, can you send Arthur the forms please? > ^^^^^ > > Alexander, obviously ... > > martin Arthur already signed and prefer borderless stuff ... :-) ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-06 11:32 ` Arthur Miller @ 2021-01-06 13:36 ` martin rudalics 2021-01-06 15:01 ` Arthur Miller 0 siblings, 1 reply; 27+ messages in thread From: martin rudalics @ 2021-01-06 13:36 UTC (permalink / raw) To: Arthur Miller; +Cc: 45620, Alexander Miller, tumashu > Arthur already signed and prefer borderless stuff ... :-) Waiting for Henry to show up ... martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-06 13:36 ` martin rudalics @ 2021-01-06 15:01 ` Arthur Miller 0 siblings, 0 replies; 27+ messages in thread From: Arthur Miller @ 2021-01-06 15:01 UTC (permalink / raw) To: martin rudalics; +Cc: 45620, Alexander Miller, tumashu martin rudalics <rudalics@gmx.at> writes: >> Arthur already signed and prefer borderless stuff ... :-) > > Waiting for Henry to show up ... > > martin And if Glenn appears we could have a jazz quartet! ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-05 15:33 ` martin rudalics 2021-01-05 15:34 ` martin rudalics @ 2021-01-05 16:26 ` Eli Zaretskii 1 sibling, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2021-01-05 16:26 UTC (permalink / raw) To: martin rudalics; +Cc: 45620, alexanderm, tumashu > Cc: 45620@debbugs.gnu.org, tumashu@163.com, Eli Zaretskii <eliz@gnu.org> > From: martin rudalics <rudalics@gmx.at> > Date: Tue, 5 Jan 2021 16:33:05 +0100 > > Eli, can you send Arthur the forms please? Done. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller ` (3 preceding siblings ...) 2021-01-05 12:50 ` Alexander Miller @ 2021-01-06 16:32 ` Alexander Miller 2021-01-06 18:48 ` martin rudalics 2021-01-13 9:17 ` Alexander Miller ` (3 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-06 16:32 UTC (permalink / raw) To: rudalics; +Cc: 45620, tumashu [-- Attachment #1: Type: text/plain, Size: 1181 bytes --] >> >> - Currently the actual width of the border is still controlled by the >> >> `internal-border-width` parameter for both frame types. Should I try to >> >> do something about that as well? If yes, what's my entry point? >> > >> > Add a 'child-frame-border-width' parameter. But in this case I would >> > propose to proceed as follows: >> > >> > - If for a frame the 'child-frame-border-width' was explicitly set, use >> > it. >> > >> > - If it was not set, use the 'internal-border-width' parameter. >> >> That's done now too, at least for X. I'll attach patches of my first >> working drafts for both changes. Let me know if I'm on the right path so >> far. > > You're already at the end of it. Please document the changes in the > Elisp manual and announce them as (probably incompatible) changes in > NEWS. > Ok, all done now, see attached patch. For the copyright I have asked my employer to make sure that the copyright parts from my contract don't apply to the work I do on my own time like this, but most of the company will still be on holidays or on sick leave, so it could be a few days before I get an answer. [-- Attachment #2: 0001-Add-distinct-controls-for-the-appearance-of-child-fr.patch --] [-- Type: text/x-patch, Size: 19453 bytes --] From 623b6ab24b93ca21e7fa32dd344955d1138c157f Mon Sep 17 00:00:00 2001 From: Alexander Miller <alexanderm@web.de> Date: Wed, 6 Jan 2021 17:02:35 +0100 Subject: [PATCH] Add distinct controls for the appearance of child frames' borders. The background of the `child-frame-border` face instead of the `internal-border` face now controls the color of child frames' borders. The `child-frame-border-width` frame parameter is now used for the width of child frames' borders instead of `internal-border-width`, though we still fall back on using the latter if the former is not set. --- doc/lispref/frames.texi | 19 +++++++++++--- etc/NEWS | 7 +++++ lisp/faces.el | 11 +++++++- src/dispextern.h | 1 + src/frame.c | 12 +++++++++ src/frame.h | 25 ++++++++++++++++-- src/nsfns.m | 18 +++++++++++++ src/nsterm.m | 10 ++++--- src/w32fns.c | 58 ++++++++++++++++++++++++++++++++++++++--- src/xfaces.c | 3 +++ src/xfns.c | 46 ++++++++++++++++++++++++++++++++ src/xterm.c | 20 +++++++++----- 12 files changed, 212 insertions(+), 18 deletions(-) diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi index 7f2a6f7542..ef1b661b2a 100644 --- a/doc/lispref/frames.texi +++ b/doc/lispref/frames.texi @@ -694,9 +694,17 @@ Frame Layout @item Internal Border The internal border is a border drawn by Emacs around the inner frame -(see below). Its width is specified by the @code{internal-border-width} -frame parameter (@pxref{Layout Parameters}). Its color is specified by -the background of the @code{internal-border} face. +(see below). The specification of its appearance depends on whether +the given frame is a child frame (@pxref{Child Frames}) or not. + +For normal frames its width is specified by the @code{internal-border-width} +frame parameter (@pxref{Layout Parameters}) and its color is specified by the +background of the @code{internal-border} face. + +For child frames its width is specified by the @code{child-frame-border-width} +frame parameter (but will use the the @code{internal-border-width} parameter as +a fallback) and its color is specified by the background of the +@code{child-frame-border} face. @item Inner Frame @cindex inner frame @@ -1790,6 +1798,11 @@ Layout Parameters The width in pixels of the frame's internal border (@pxref{Frame Geometry}). +@vindex child-frame-border-width@r{, a frame parameter} +@item child-frame-border-width +The width in pixels of the frame's internal border (@pxref{Frame +Geometry}) if the given frame is a child frame (@pxref{Child Frames}). + @vindex vertical-scroll-bars@r{, a frame parameter} @item vertical-scroll-bars Whether the frame has scroll bars (@pxref{Scroll Bars}) for vertical diff --git a/etc/NEWS b/etc/NEWS index d8f25ab362..f1e8443080 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -218,6 +218,13 @@ and other "slow scrolling" situations. It is hoped it behaves better than 'fast-but-imprecise-scrolling' and 'jit-lock-defer-time'. It is not enabled by default. +** New face 'child-frame-border' and new frame parameter child-frame-border-width +The face and width of child frames can no be determined separately from normal +frames. To minimize backwards incompatibility child-frames without a +'child-frame-border-width' parameter will fallback to using 'internal-border-width'. +However the new 'child-frame-border' face does constitute a breaking change since +child frames' borders no longer use the 'internal-border' face. + \f * Editing Changes in Emacs 28.1 diff --git a/lisp/faces.el b/lisp/faces.el index 4e98338432..639dbeb09a 100644 --- a/lisp/faces.el +++ b/lisp/faces.el @@ -2683,11 +2683,20 @@ window-divider-last-pixel (defface internal-border '((t nil)) - "Basic face for the internal border." + "Basic face for the internal border. +For the internal border of child frames see `child-frame-border'." :version "26.1" :group 'frames :group 'basic-faces) +(defface child-frame-border + '((t nil)) + "Basic face for the internal border of child frames. +For the internal border of non-child frames see `internal-border'." + :version "28.1" + :group 'frames + :group 'basic-faces) + (defface minibuffer-prompt '((((background dark)) :foreground "cyan") ;; Don't use blue because many users of the MS-DOS port customize diff --git a/src/dispextern.h b/src/dispextern.h index 3ad98b8344..f4e872644d 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -1826,6 +1826,7 @@ #define FACE_UNIBYTE_P(FACE) ((FACE)->charset < 0) WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID, WINDOW_DIVIDER_LAST_PIXEL_FACE_ID, INTERNAL_BORDER_FACE_ID, + CHILD_FRAME_BORDER_FACE_ID, TAB_BAR_FACE_ID, TAB_LINE_FACE_ID, BASIC_FACE_ID_SENTINEL diff --git a/src/frame.c b/src/frame.c index 45ee96e962..4a4ad99395 100644 --- a/src/frame.c +++ b/src/frame.c @@ -3528,6 +3528,13 @@ DEFUN ("frame-fringe-width", Ffringe_width, Sfringe_width, 0, 1, 0, return make_fixnum (FRAME_TOTAL_FRINGE_WIDTH (decode_any_frame (frame))); } +DEFUN ("frame-child-frame-border-width", Fframe_child_frame_border_width, Sframe_child_frame_border_width, 0, 1, 0, + doc: /* Return width of FRAME's child-frame border in pixels. */) + (Lisp_Object frame) +{ + return make_fixnum (FRAME_CHILD_FRAME_BORDER_WIDTH (decode_any_frame (frame))); +} + DEFUN ("frame-internal-border-width", Fframe_internal_border_width, Sframe_internal_border_width, 0, 1, 0, doc: /* Return width of FRAME's internal border in pixels. */) (Lisp_Object frame) @@ -3744,6 +3751,7 @@ DEFUN ("set-frame-window-state-change", Fset_frame_window_state_change, {"foreground-color", -1}, {"icon-name", SYMBOL_INDEX (Qicon_name)}, {"icon-type", SYMBOL_INDEX (Qicon_type)}, + {"child-frame-border-width", SYMBOL_INDEX (Qchild_frame_border_width)}, {"internal-border-width", SYMBOL_INDEX (Qinternal_border_width)}, {"right-divider-width", SYMBOL_INDEX (Qright_divider_width)}, {"bottom-divider-width", SYMBOL_INDEX (Qbottom_divider_width)}, @@ -4287,6 +4295,8 @@ gui_report_frame_params (struct frame *f, Lisp_Object *alistptr) store_in_alist (alistptr, Qborder_width, make_fixnum (f->border_width)); + store_in_alist (alistptr, Qchild_frame_border_width, + make_fixnum (FRAME_CHILD_FRAME_BORDER_WIDTH (f))); store_in_alist (alistptr, Qinternal_border_width, make_fixnum (FRAME_INTERNAL_BORDER_WIDTH (f))); store_in_alist (alistptr, Qright_divider_width, @@ -5984,6 +5994,7 @@ syms_of_frame (void) DEFSYM (Qhorizontal_scroll_bars, "horizontal-scroll-bars"); DEFSYM (Qicon_name, "icon-name"); DEFSYM (Qicon_type, "icon-type"); + DEFSYM (Qchild_frame_border_width, "child-frame-border-width"); DEFSYM (Qinternal_border_width, "internal-border-width"); DEFSYM (Qleft_fringe, "left-fringe"); DEFSYM (Qline_spacing, "line-spacing"); @@ -6408,6 +6419,7 @@ focus (where a frame immediately loses focus when it's left by the mouse defsubr (&Sscroll_bar_width); defsubr (&Sscroll_bar_height); defsubr (&Sfringe_width); + defsubr (&Sframe_child_frame_border_width); defsubr (&Sframe_internal_border_width); defsubr (&Sright_divider_width); defsubr (&Sbottom_divider_width); diff --git a/src/frame.h b/src/frame.h index 8cf41dc004..6a94cee727 100644 --- a/src/frame.h +++ b/src/frame.h @@ -534,6 +534,10 @@ #define EMACS_FRAME_H /* Border width of the frame window as known by the (X) window system. */ int border_width; + /* Width of child frames' internal border. Acts as the + interal_border_width foy child frames. */ + int child_frame_border_width; + /* Width of the internal border. This is a line of background color just inside the window's border. When the frame is selected, a highlighting is displayed inside the internal border. */ @@ -1432,11 +1436,28 @@ FRAME_TOTAL_FRINGE_WIDTH (struct frame *f) return FRAME_LEFT_FRINGE_WIDTH (f) + FRAME_RIGHT_FRINGE_WIDTH (f); } -/* Pixel-width of internal border lines. */ +INLINE int +FRAME_CHILD_FRAME_BORDER_WIDTH (struct frame *f) +{ + return frame_dimension (f->child_frame_border_width); +} + +/* Pixel-width of internal border lines. + Will use child_frame_border_width for child frames if + possible, and fall back on internal_border_width + otherwise. */ INLINE int FRAME_INTERNAL_BORDER_WIDTH (struct frame *f) { - return frame_dimension (f->internal_border_width); +#ifdef HAVE_WINDOW_SYSTEM + return FRAME_PARENT_FRAME(f) + ? (f->child_frame_border_width + ? FRAME_CHILD_FRAME_BORDER_WIDTH(f) + : frame_dimension (f->internal_border_width)) + : frame_dimension (f->internal_border_width); +#else + return frame_dimension (f->internal_border_width) +#endif } /* Pixel-size of window divider lines. */ diff --git a/src/nsfns.m b/src/nsfns.m index ae114f83e4..1a81b07aae 100644 --- a/src/nsfns.m +++ b/src/nsfns.m @@ -687,6 +687,21 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. } } +static void +ns_set_child_frame_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) +{ + int old_width = FRAME_CHILD_FRAME_BORDER_WIDTH (f); + int new_width = check_int_nonnegative (arg); + + if (new_width == old_width) + return; + f->child_frame_border_width = new_width; + + if (FRAME_NATIVE_WINDOW (f) != 0) + adjust_frame_size (f, -1, -1, 3, 0, Qchild_frame_border_width); + + SET_FRAME_GARBAGED (f); +} static void ns_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) @@ -1197,6 +1212,9 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. gui_default_parameter (f, parms, Qinternal_border_width, make_fixnum (2), "internalBorderWidth", "InternalBorderWidth", RES_TYPE_NUMBER); + gui_default_parameter (f, parms, Qchild_frame_border_width, make_fixnum (2), + "childFrameBorderWidth", "childFrameBorderWidth", + RES_TYPE_NUMBER); gui_default_parameter (f, parms, Qright_divider_width, make_fixnum (0), NULL, NULL, RES_TYPE_NUMBER); gui_default_parameter (f, parms, Qbottom_divider_width, make_fixnum (0), diff --git a/src/nsterm.m b/src/nsterm.m index 2defb9e2ee..76f67d2531 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -3029,9 +3029,13 @@ so some key presses (TAB) are swallowed by the system. */ NSRectEdge edge[] = {NSMinXEdge, NSMinYEdge, NSMaxXEdge, NSMaxYEdge}; int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); if (!face) diff --git a/src/w32fns.c b/src/w32fns.c index c1e18ff7fa..1d3fb5d761 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -1519,9 +1519,13 @@ w32_clear_under_internal_border (struct frame *f) int width = FRAME_PIXEL_WIDTH (f); int height = FRAME_PIXEL_HEIGHT (f); int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); block_input (); @@ -1548,6 +1552,32 @@ w32_clear_under_internal_border (struct frame *f) } } +/** + * w32_set_child_frame_border_width: + * + * Set width of child frame F's internal border to ARG pixels. + * ARG < 0 is * treated like ARG = 0. + */ +static void +w32_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) +{ + int argval = check_integer_range (arg, INT_MIN, INT_MAX); + int border = max (argval, 0); + + if (border != FRAME_CHILD_FRAME_BORDER_WIDTH (f)) + { + f->child_frame_border_width = border; + + if (FRAME_NATIVE_WINDOW (f) != 0) + { + adjust_frame_size (f, -1, -1, 3, false, Qchild_frame_border_width); + + if (FRAME_VISIBLE_P (f)) + w32_clear_under_internal_border (f); + } + } +} + /** * w32_set_internal_border_width: @@ -5873,6 +5903,28 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame, parameters); } + /* Same for child frames. */ + if (NILP (Fassq (Qchild_frame_border_width, parameters))) + { + Lisp_Object value; + + value = gui_display_get_arg (dpyinfo, parameters, Qchild_frame_border_width, + "childFrameBorderWidth", "childFrameBorderWidth", + RES_TYPE_NUMBER); + if (! EQ (value, Qunbound)) + parameters = Fcons (Fcons (Qchild_frame_border_width, value), + parameters); + + } + + gui_default_parameter (f, parameters, Qchild_frame_border_width, +#ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets. */ + make_fixnum (0), +#else + make_fixnum (1), +#endif + "childFrameBorderWidth", "childFrameBorderWidth", + RES_TYPE_NUMBER); gui_default_parameter (f, parameters, Qinternal_border_width, make_fixnum (0), "internalBorderWidth", "InternalBorder", RES_TYPE_NUMBER); gui_default_parameter (f, parameters, Qright_divider_width, make_fixnum (0), diff --git a/src/xfaces.c b/src/xfaces.c index b3b19a9cb2..5080120957 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -4913,6 +4913,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id) case WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID: name = Qwindow_divider_first_pixel; break; case WINDOW_DIVIDER_LAST_PIXEL_FACE_ID: name = Qwindow_divider_last_pixel; break; case INTERNAL_BORDER_FACE_ID: name = Qinternal_border; break; + case CHILD_FRAME_BORDER_FACE_ID: name = Qchild_frame_border; break; default: emacs_abort (); /* the caller is supposed to pass us a basic face id */ @@ -5619,6 +5620,7 @@ realize_basic_faces (struct frame *f) realize_named_face (f, Qwindow_divider_last_pixel, WINDOW_DIVIDER_LAST_PIXEL_FACE_ID); realize_named_face (f, Qinternal_border, INTERNAL_BORDER_FACE_ID); + realize_named_face (f, Qchild_frame_border, CHILD_FRAME_BORDER_FACE_ID); realize_named_face (f, Qtab_bar, TAB_BAR_FACE_ID); realize_named_face (f, Qtab_line, TAB_LINE_FACE_ID); @@ -6967,6 +6969,7 @@ syms_of_xfaces (void) DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel"); DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel"); DEFSYM (Qinternal_border, "internal-border"); + DEFSYM (Qchild_frame_border, "child-frame-border"); /* TTY color-related functions (defined in tty-colors.el). */ DEFSYM (Qtty_color_desc, "tty-color-desc"); diff --git a/src/xfns.c b/src/xfns.c index 9ab537ca8d..cac41ee485 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -1800,6 +1800,28 @@ x_change_tool_bar_height (struct frame *f, int height) #endif /* USE_GTK */ } +static void +x_set_child_frame_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) +{ + int border = check_int_nonnegative (arg); + + if (border != FRAME_CHILD_FRAME_BORDER_WIDTH (f)) + { + f->child_frame_border_width = border; + +#ifdef USE_X_TOOLKIT + if (FRAME_X_OUTPUT (f)->edit_widget) + widget_store_internal_border (FRAME_X_OUTPUT (f)->edit_widget); +#endif + + if (FRAME_X_WINDOW (f)) + { + adjust_frame_size (f, -1, -1, 3, false, Qchild_frame_border_width); + x_clear_under_internal_border (f); + } + } + +} static void x_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) @@ -3897,6 +3919,29 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame, parms = Fcons (Fcons (Qinternal_border_width, value), parms); } + + /* Same for child frames. */ + if (NILP (Fassq (Qchild_frame_border_width, parms))) + { + Lisp_Object value; + + value = gui_display_get_arg (dpyinfo, parms, Qchild_frame_border_width, + "childFrameBorderWidth", "childFrameBorderWidth", + RES_TYPE_NUMBER); + if (! EQ (value, Qunbound)) + parms = Fcons (Fcons (Qchild_frame_border_width, value), + parms); + + } + + gui_default_parameter (f, parms, Qchild_frame_border_width, +#ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets. */ + make_fixnum (0), +#else + make_fixnum (1), +#endif + "childFrameBorderWidth", "childFrameBorderWidth", + RES_TYPE_NUMBER); gui_default_parameter (f, parms, Qinternal_border_width, #ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets. */ make_fixnum (0), @@ -7762,6 +7807,7 @@ DEFUN ("x-gtk-debug", Fx_gtk_debug, Sx_gtk_debug, 1, 1, 0, x_set_foreground_color, x_set_icon_name, x_set_icon_type, + x_set_child_frame_border_width, x_set_internal_border_width, gui_set_right_divider_width, gui_set_bottom_divider_width, diff --git a/src/xterm.c b/src/xterm.c index 0a86738cc2..92dcd95043 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -1293,9 +1293,13 @@ x_clear_under_internal_border (struct frame *f) int height = FRAME_PIXEL_HEIGHT (f); int margin = FRAME_TOP_MARGIN_HEIGHT (f); int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); block_input (); @@ -1360,9 +1364,13 @@ x_after_update_window_line (struct window *w, struct glyph_row *desired_row) { int y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, desired_row->y)); int face_id = - !NILP (Vface_remapping_alist) - ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) - : INTERNAL_BORDER_FACE_ID; + (FRAME_PARENT_FRAME (f) + ? (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID) + : CHILD_FRAME_BORDER_FACE_ID) + : (!NILP (Vface_remapping_alist) + ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID) + : INTERNAL_BORDER_FACE_ID)); struct face *face = FACE_FROM_ID_OR_NULL (f, face_id); block_input (); -- 2.29.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-06 16:32 ` Alexander Miller @ 2021-01-06 18:48 ` martin rudalics 0 siblings, 0 replies; 27+ messages in thread From: martin rudalics @ 2021-01-06 18:48 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > Ok, all done now, see attached patch. Looks good. > For the copyright I have asked my employer to make sure that the > copyright parts from my contract don't apply to the work I do on my own > time like this, but most of the company will still be on holidays or on > sick > leave, so it could be a few days before I get an answer. Let's hope for the best. Thanks, martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller ` (4 preceding siblings ...) 2021-01-06 16:32 ` Alexander Miller @ 2021-01-13 9:17 ` Alexander Miller 2021-01-13 18:07 ` martin rudalics 2021-01-25 12:08 ` Alexander Miller ` (2 subsequent siblings) 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-13 9:17 UTC (permalink / raw) To: rudalics; +Cc: 45620, alexanderm, tumashu >> For the copyright I have asked my employer to make sure that the >> copyright parts from my contract don't apply to the work I do on my own >> time like this, but most of the company will still be on holidays or on >> sick >> leave, so it could be a few days before I get an answer. > > Let's hope for the best. > > Thanks, martin I got green light from my employer and sent the signed papers today - we're good to go for whatever comes next. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-13 9:17 ` Alexander Miller @ 2021-01-13 18:07 ` martin rudalics 0 siblings, 0 replies; 27+ messages in thread From: martin rudalics @ 2021-01-13 18:07 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > I got green light from my employer and sent the signed papers today - > we're good to go for whatever comes next. Fine. When the FSF gets back to you please tell us. Thanks, martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller ` (5 preceding siblings ...) 2021-01-13 9:17 ` Alexander Miller @ 2021-01-25 12:08 ` Alexander Miller 2021-01-25 19:05 ` martin rudalics 2021-01-27 20:44 ` Alexander Miller 2021-01-28 7:06 ` Alexander Miller 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-25 12:08 UTC (permalink / raw) To: rudalics; +Cc: 45620, alexanderm, tumashu >> I got green light from my employer and sent the signed papers today - >> we're good to go for whatever comes next. > > Fine. When the FSF gets back to you please tell us. > > Thanks, martin They got back to me, I have received my signed copy of the documents now. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-25 12:08 ` Alexander Miller @ 2021-01-25 19:05 ` martin rudalics 2021-01-26 15:59 ` martin rudalics 0 siblings, 1 reply; 27+ messages in thread From: martin rudalics @ 2021-01-25 19:05 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > They got back to me, I have received my signed copy of the documents now. I hope to install your patch tomorrow. If you want to ensure a precise position for the NEWS entry, you'd have to update it though. Otherwise, I'll try to find a suitable position myself. Thanks, martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-25 19:05 ` martin rudalics @ 2021-01-26 15:59 ` martin rudalics 2021-01-27 20:49 ` Alan Third 0 siblings, 1 reply; 27+ messages in thread From: martin rudalics @ 2021-01-26 15:59 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > I hope to install your patch tomorrow. If you want to ensure a precise > position for the NEWS entry, you'd have to update it though. Otherwise, > I'll try to find a suitable position myself. I pushed the changes now with a few tweaks. Please have a look and consider closing the bug if you feel that nothing else should be done. I couldn't test the changes with GNUstep here, those builds still fail to behave reasonably on my machine. So if you know people who regularly build Emacs with NS, please ask them to check the changes. Thanks, martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-26 15:59 ` martin rudalics @ 2021-01-27 20:49 ` Alan Third 2021-01-28 9:42 ` martin rudalics 0 siblings, 1 reply; 27+ messages in thread From: Alan Third @ 2021-01-27 20:49 UTC (permalink / raw) To: martin rudalics; +Cc: 45620, Alexander Miller, tumashu On Tue, Jan 26, 2021 at 04:59:13PM +0100, martin rudalics wrote: > > I hope to install your patch tomorrow. If you want to ensure a precise > > position for the NEWS entry, you'd have to update it though. Otherwise, > > I'll try to find a suitable position myself. > > I pushed the changes now with a few tweaks. Please have a look and > consider closing the bug if you feel that nothing else should be done. > > I couldn't test the changes with GNUstep here, those builds still fail > to behave reasonably on my machine. So if you know people who regularly > build Emacs with NS, please ask them to check the changes. Is there some simple test I can try? I had a quick look through the thread but didn't see anything. (GNUstep builds work for me as long as I start with a .emacs that turns off the menus. I can't work out what it is that's causing the problem, and even the large rewrite of some of the menu code we did on the master branch makes no difference. It's quite annoying.) -- Alan Third ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-27 20:49 ` Alan Third @ 2021-01-28 9:42 ` martin rudalics 2021-01-28 16:35 ` Alan Third 0 siblings, 1 reply; 27+ messages in thread From: martin rudalics @ 2021-01-28 9:42 UTC (permalink / raw) To: Alan Third, Alexander Miller, 45620, tumashu > Is there some simple test I can try? I had a quick look through the > thread but didn't see anything. Please with emacs -Q define (defun my-make-child-frame () (interactive) (make-frame `((parent-frame . ,(selected-frame)) (undecorated . t) (left . 0.5) (top . 0.5) (width . 0.3) (height . 0.3) (internal-border-width . 3)))) then do (setq frame (my-make-child-frame)) and finally perform the following two experiments: (1) Customize the background of the faces 'internal-border' and 'child-frame-border'. Only customizing the latter should affect the child frame. (2) Do (set-frame-parameter frame 'internal-border-width 7) (set-frame-parameter frame 'child-frame-border-width 1) Either of them should change the border width of the child frame as indicated. If you now do (set-frame-parameter nil 'internal-border-width 5) with your normal frame selected, that frame's internal border width should change but the child frame's border width should remain unaltered. > (GNUstep builds work for me as long as I start with a .emacs that > turns off the menus. I can't work out what it is that's causing the > problem, and even the large rewrite of some of the menu code we did on > the master branch makes no difference. It's quite annoying.) I'll try turning menus off the next time. But the number of warnings when building has by now exceeded any reasonable limit. I can no more see the wood for the trees Thanks, martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-28 9:42 ` martin rudalics @ 2021-01-28 16:35 ` Alan Third 2021-01-29 7:51 ` martin rudalics 0 siblings, 1 reply; 27+ messages in thread From: Alan Third @ 2021-01-28 16:35 UTC (permalink / raw) To: martin rudalics; +Cc: 45620, Alexander Miller, tumashu On Thu, Jan 28, 2021 at 10:42:18AM +0100, martin rudalics wrote: > > Is there some simple test I can try? I had a quick look through the > > thread but didn't see anything. > > Please with emacs -Q define > > (defun my-make-child-frame () > (interactive) > (make-frame > `((parent-frame . ,(selected-frame)) > (undecorated . t) > (left . 0.5) > (top . 0.5) > (width . 0.3) > (height . 0.3) > (internal-border-width . 3)))) > > then do > > (setq frame (my-make-child-frame)) > > and finally perform the following two experiments: > > (1) Customize the background of the faces 'internal-border' and > 'child-frame-border'. Only customizing the latter should affect the > child frame. > > (2) Do > > (set-frame-parameter frame 'internal-border-width 7) > (set-frame-parameter frame 'child-frame-border-width 1) > > Either of them should change the border width of the child frame as > indicated. If you now do > > (set-frame-parameter nil 'internal-border-width 5) > > with your normal frame selected, that frame's internal border width > should change but the child frame's border width should remain > unaltered. That all works as I expect on macOS, so I guess we can say it's fine? > > (GNUstep builds work for me as long as I start with a .emacs that > > turns off the menus. I can't work out what it is that's causing the > > problem, and even the large rewrite of some of the menu code we did on > > the master branch makes no difference. It's quite annoying.) > > I'll try turning menus off the next time. But the number of warnings > when building has by now exceeded any reasonable limit. I can no more > see the wood for the trees Ah yes, I'd forgotten about that. As I recall the errors are mostly from the GNUstep headers, so I don't know if they're caused by some incompatibility between recent versions of GCC and GNUstep, or if we're setting some compiler flag that GNUstep dislikes. I don't get these warnings on my old Debian Jessie build environment, just on the newer Buster one, but Jessie's really old by now. -- Alan Third ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-28 16:35 ` Alan Third @ 2021-01-29 7:51 ` martin rudalics 0 siblings, 0 replies; 27+ messages in thread From: martin rudalics @ 2021-01-29 7:51 UTC (permalink / raw) To: Alan Third, Alexander Miller, 45620, tumashu > That all works as I expect on macOS, so I guess we can say it's fine? I guess so too. Many thanks for checking. > I don't get these warnings on my old Debian Jessie build environment, > just on the newer Buster one, but Jessie's really old by now. Here (with Buster) I especially get some 2000 lines of 2021-01-29 08:45:54.042 temacs[3778:3778] autorelease called without pool for object (0xd6f990) of class GSCInlineString in thread <NSThread: 0xbde000>{name = (null), num = 3778} I've never seen before. martin ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller ` (6 preceding siblings ...) 2021-01-25 12:08 ` Alexander Miller @ 2021-01-27 20:44 ` Alexander Miller 2021-01-28 3:33 ` Eli Zaretskii 2021-01-28 7:06 ` Alexander Miller 8 siblings, 1 reply; 27+ messages in thread From: Alexander Miller @ 2021-01-27 20:44 UTC (permalink / raw) To: rudalics; +Cc: 45620, tumashu >> I hope to install your patch tomorrow. If you want to ensure a precise >> position for the NEWS entry, you'd have to update it though. Otherwise, >> I'll try to find a suitable position myself. > > I pushed the changes now with a few tweaks. Please have a look and > consider closing the bug if you feel that nothing else should be done. I have tested the changes locally now, and everything is working as expected. I'd close the bug, but I don't know how. I have found some 12 year old documentation that says to send my mail to nnn-done@debbugs.gnu.org. Is that still correct, or will my mail just get lost? > I couldn't test the changes with GNUstep here, those builds still fail > to behave reasonably on my machine. So if you know people who regularly > build Emacs with NS, please ask them to check the changes. I have gotten one or two people into Emacs to some degree over the years, but I am the most hardcore user I know by far, so I'm afraid I can't help with that. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-27 20:44 ` Alexander Miller @ 2021-01-28 3:33 ` Eli Zaretskii 0 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2021-01-28 3:33 UTC (permalink / raw) To: Alexander Miller; +Cc: 45620, tumashu > From: Alexander Miller <alexanderm@web.de> > Date: Wed, 27 Jan 2021 21:44:21 +0100 > Cc: 45620@debbugs.gnu.org, tumashu@163.com > > I'd close the bug, but I don't know how. I have found some 12 > year old documentation that says to send my mail to > nnn-done@debbugs.gnu.org. Is that still correct, or will my mail just > get lost? This is still correct. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#45620: 28.0.50; Child frames should have their own border width and colour 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller ` (7 preceding siblings ...) 2021-01-27 20:44 ` Alexander Miller @ 2021-01-28 7:06 ` Alexander Miller 8 siblings, 0 replies; 27+ messages in thread From: Alexander Miller @ 2021-01-28 7:06 UTC (permalink / raw) To: eliz; +Cc: 45620-done, alexanderm, tumashu >> From: Alexander Miller <alexanderm@web.de> >> Date: Wed, 27 Jan 2021 21:44:21 +0100 >> Cc: 45620@debbugs.gnu.org, tumashu@163.com >> >> I'd close the bug, but I don't know how. I have found some 12 >> year old documentation that says to send my mail to >> nnn-done@debbugs.gnu.org. Is that still correct, or will my mail just >> get lost? > > This is still correct. All right, let's close this bug then. ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2021-01-29 7:51 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-03 13:24 bug#45620: 28.0.50; Child frames should have their own border width and colour Alexander Miller 2021-01-03 16:15 ` martin rudalics 2021-01-04 13:38 ` Alexander Miller 2021-01-04 16:22 ` martin rudalics 2021-01-04 17:48 ` Alexander Miller 2021-01-04 18:54 ` martin rudalics 2021-01-05 12:50 ` Alexander Miller 2021-01-05 15:33 ` martin rudalics 2021-01-05 15:34 ` martin rudalics 2021-01-06 11:32 ` Arthur Miller 2021-01-06 13:36 ` martin rudalics 2021-01-06 15:01 ` Arthur Miller 2021-01-05 16:26 ` Eli Zaretskii 2021-01-06 16:32 ` Alexander Miller 2021-01-06 18:48 ` martin rudalics 2021-01-13 9:17 ` Alexander Miller 2021-01-13 18:07 ` martin rudalics 2021-01-25 12:08 ` Alexander Miller 2021-01-25 19:05 ` martin rudalics 2021-01-26 15:59 ` martin rudalics 2021-01-27 20:49 ` Alan Third 2021-01-28 9:42 ` martin rudalics 2021-01-28 16:35 ` Alan Third 2021-01-29 7:51 ` martin rudalics 2021-01-27 20:44 ` Alexander Miller 2021-01-28 3:33 ` Eli Zaretskii 2021-01-28 7:06 ` Alexander Miller
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).