GNU bug report logs - #59660
29.0.50; typescript-ts-mode consistently fontifies method-names incorrectly

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59660 in the body.
You can then email your comments to 59660 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>,
 Theodor Thornhill <theo <at> thornhill.no>, Yuan Fu <casouri <at> gmail.com>
Subject: 29.0.50; typescript-ts-mode consistently fontifies method-names
 incorrectly
Date: Mon, 28 Nov 2022 16:04:41 +0100

[Message part 1 (text/plain, inline)]

From: "Jostein Kjønigsen" 
<jostein <at> ThinkPad-T14s.mail-host-address-is-not-set>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; typescript-ts-mode consistently fontifies method-names 
incorrectly
--text follows this line--

--
When I:

1. create a typescript file (sample.ts)
2. create class and add some methods to it.

I observe that:

- method names are consistently fontified as font-lock-property-face.

I expected that:

- method names to be consistently fontified as font-luck-function-name-face.

This seems to be because of the "property_identifier" is used for
functions in the treesitter-grammar, and this property is used
indiscriminately to apply font-lock-property-face at the bottom of the
major-mode grammar.

This "shadows" the former rules which correctly apply 
font-lock-function-name-face.

The best would obviously have a way to identify properties more 
accurately, to precent
shadowing from taking place. Unfortunately I cannot see a realiable way 
to do that
based on the current parse-tree.

Seeing as properties in Typescript are fairly rare while methods are 
very common,
it seems appropriate to me to remove the property-specific fontification 
rules
to restore the method-fontification rules.

I got a patch for this ready, if people are willing to accept it.

--


In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.34, cairo version 1.16.0) of 2022-11-28 built on ThinkPad-T14s
Repository revision: a85ff22300736212e38f43cc7d56e8e3d4ae1203
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12201003
System Description: Ubuntu 22.10

Configured using:
 'configure --with-tree-sitter'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: nb_NO.UTF-8
  value of $LC_NUMERIC: nb_NO.UTF-8
  value of $LC_TIME: nb_NO.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: TypeScript

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  lsp-diagnostics-mode: t
  lsp-headerline-breadcrumb-mode: t
  lsp-modeline-workspace-status-mode: t
  lsp-modeline-diagnostics-mode: t
  lsp-modeline-code-actions-mode: t
  electric-pair-mode: t
  lsp-completion-mode: t
  editorconfig-mode: t
  flycheck-mode: t
  which-function-mode: t
  nlinum-mode: t
  company-mode: t
  global-ede-mode: t
  dap-tooltip-mode: t
  dap-ui-many-windows-mode: t
  dap-ui-controls-mode: t
  dap-ui-mode: t
  treemacs-filewatch-mode: t
  treemacs-follow-mode: t
  treemacs-git-mode: t
  treemacs-fringe-indicator-mode: t
  dap-auto-configure-mode: t
  dap-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  doom-modeline-mode: t
  projectile-mode: t
  ido-yes-or-no-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  helm--remap-mouse-mode: t
  async-bytecomp-package-mode: t
  delete-selection-mode: t
  global-auto-revert-mode: t
  server-mode: t
  shell-dirtrack-mode: t
  global-hl-line-mode: t
  lsp-managed-mode: t
  lsp-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: yas--auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/jostein/.emacs.d/elpa/transient-20221028.1430/transient hides 
/home/jostein/build/emacs/lisp/transient
/home/jostein/.emacs.d/elpa/eglot-20221020.1010/eglot hides 
/home/jostein/build/emacs/lisp/progmodes/eglot

Features:
(cl-print git-rebase shadow sort emacsbug magit-extras mail-extr
flyspell ispell mule-util tree-sitter-langs tree-sitter-langs-build
tar-mode arc-mode archive-mode tree-sitter-hl tree-sitter-debug
tree-sitter tree-sitter-load tree-sitter-cli tsc tsc-dyn tsc-dyn-get
dired-aux tsc-obsolete misearch multi-isearch semantic/lex-spp ede/emacs
semantic/db semantic/util-modes semantic/util semantic semantic/tag
semantic/lex semantic/fw mode-local bug-reference magit-submodule
magit-obsolete magit-blame magit-stash magit-reflog magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode
git-commit log-edit message sendmail yank-media rfc822 mml mml-sec epa
mm-decode mm-bodies mm-encode mailabbrev gmm-utils mailheader pcvs-util
magit-core magit-autorevert magit-margin magit-transient magit-process
with-editor magit-mode transient magit-git magit-base magit-section crm
compat-27 compat-26 executable helm-command helm-elisp helm-eval
helm-info url-http url-auth mail-parse rfc2231 rfc2047 rfc2045
ietf-drums url-gw face-remap lsp-diagnostics lsp-headerline lsp-icons
lsp-modeline view vc-git diff-mode vc-dispatcher disp-table elec-pair
winner ffap tramp-archive tramp-gvfs tramp-cache warnings time-stamp
zeroconf dbus add-log lsp-zig lsp-steep lsp-svelte lsp-sqls
lsp-ruby-syntax-tree lsp-yaml lsp-xml lsp-vimscript lsp-vhdl lsp-volar
lsp-vetur lsp-html lsp-verilog lsp-vala lsp-v lsp-typeprof lsp-ttcn3
lsp-toml lsp-terraform lsp-tex lsp-sorbet lsp-solargraph lsp-rust lsp-rf
lsp-remark lsp-racket lsp-r lsp-purescript lsp-pylsp lsp-pyls lsp-php
lsp-pls lsp-perlnavigator lsp-perl lsp-openscad lsp-ocaml lsp-magik
lsp-nix lsp-nim lsp-nginx lsp-mint lsp-marksman lsp-markdown lsp-lua
lsp-kotlin lsp-json lsp-javascript lsp-idris lsp-haxe lsp-groovy
lsp-hack lsp-graphql lsp-gleam lsp-go lsp-completion lsp-gdscript
lsp-fsharp lsp-fortran lsp-eslint lsp-erlang lsp-emmet lsp-elixir
lsp-elm lsp-dockerfile lsp-dhall lsp-d lsp-css lsp-csharp gnutls
lsp-crystal lsp-cmake lsp-clojure lsp-semantic-tokens lsp-clangd
lsp-beancount lsp-bash lsp-ansible lsp-angular lsp-ada lsp-actionscript
ido-completing-read+ memoize minibuf-eldef elisp-slime-nav paredit
editorconfig editorconfig-core editorconfig-core-handle
editorconfig-fnmatch flycheck highlight-symbol which-func edebug debug
backtrace nlinum linum company-oddmuse company-keywords company-etags
etags fileloop generator company-gtags company-dabbrev-code
company-dabbrev company-files company-clang company-capf company-cmake
company-semantic company-template company-bbdb company-web-html
company-web company-css web-completion-data company eww url-queue shr
pixel-fill kinsoku url-file svg mm-url gnus nnheader gnus-util
mail-utils range mm-util mail-prsvr ede/speedbar ede/files ede
ede/detect ede/base ede/auto ede/source eieio-base eieio-speedbar
speedbar ezimage dframe eieio-custom cedet dap-mouse dap-ui lsp-treemacs
lsp-treemacs-generic lsp-treemacs-themes treemacs-treelib treemacs
treemacs-header-line treemacs-compatibility treemacs-mode
treemacs-interface treemacs-persistence treemacs-filewatch-mode
treemacs-follow-mode treemacs-rendering treemacs-annotations
treemacs-async treemacs-workspaces treemacs-dom treemacs-visuals
treemacs-fringe-indicator treemacs-scope pulse treemacs-faces
treemacs-icons treemacs-themes treemacs-core-utils pfuture
treemacs-logging treemacs-customization treemacs-macros gdb-mi bindat
gud bui bui-list bui-info bui-entry bui-core bui-history bui-button
bui-utils lsp-lens dap-gdb-lldb dap-netcore dap-node dap-utils dom xml
dap-pwsh lsp-pwsh dap-python dap-mode dap-tasks dap-launch lsp-docker
yaml posframe dap-overlays undo-tree diff queue doom-modeline
doom-modeline-segments doom-modeline-env doom-modeline-core
all-the-icons all-the-icons-faces data-material data-weathericons
data-octicons data-fileicons data-faicons data-alltheicons shrink-path
compat compat-macs projectile lisp-mnt grep ibuf-ext ibuffer
ibuffer-loaddefs helm-imenu ob-plantuml 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 org-version ob-emacs-lisp ob-core
ob-eval org-table oc-basic bibtex ol org-keys oc org-compat org-macs
org-loaddefs find-func cal-menu calendar cal-loaddefs ido-yes-or-no ido
helm-mode helm-misc helm-files image-dired image-dired-tags
image-dired-external image-dired-util xdg image-mode dired
dired-loaddefs exif tramp tramp-loaddefs trampver tramp-integration
cus-edit pp cus-load files-x tramp-compat parse-time iso8601 time-date
ls-lisp helm-buffers helm-occur helm-tags helm-locate helm-grep
helm-regexp format-spec helm-utils helm-help helm-types helm
helm-global-bindings helm-easymenu edmacro kmacro helm-core easy-mmode
async-bytecomp helm-source helm-multi-match helm-lib async helm-config
delsel cl-extra autorevert typescript-ts-mode js derived csharp-mode
treesit cc-langs cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs server powershell advice shell
pcomplete hl-line lsp-mode lsp-protocol yasnippet help-mode xref project
tree-widget wid-edit spinner pcase network-stream puny nsm markdown-mode
color thingatpt noutline outline icons lv inline imenu ht filenotify f
f-shortdoc shortdoc s ewoc epg rfc6068 epg-config dash dracula-theme
compile-eslint compile text-property-search comint ansi-osc ansi-color
ring cl finder-inf rx helm-projectile-autoloads expand-region-autoloads
all-the-icons-autoloads helm-autoloads dracula-theme-autoloads
eglot-autoloads nlinum-autoloads rust-mode-autoloads
multiple-cursors-autoloads magit-autoloads magit-section-autoloads
doom-modeline-autoloads assess-autoloads m-buffer-autoloads
cargo-autoloads package-lint-autoloads company-autoloads
web-mode-autoloads flycheck-autoloads git-commit-autoloads
with-editor-autoloads git-timemachine-autoloads js2-mode-autoloads
projectile-autoloads yaml-mode-autoloads helpful-autoloads
elisp-refs-autoloads powershell-autoloads helm-core-autoloads
async-autoloads editorconfig-autoloads dap-mode-autoloads
lsp-docker-autoloads yaml-autoloads lsp-treemacs-autoloads
lsp-mode-autoloads markdown-mode-autoloads transient-autoloads
compat-autoloads paredit-autoloads pcache-autoloads f-autoloads
popup-autoloads tree-sitter-langs-autoloads treemacs-autoloads
posframe-autoloads ht-autoloads hydra-autoloads pfuture-autoloads
ace-window-autoloads avy-autoloads s-autoloads info dash-autoloads
macrostep-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip
cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode 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 lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine 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 emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 769231 98943)
 (symbols 48 61705 77)
 (strings 32 224824 12295)
 (string-bytes 1 6961649)
 (vectors 16 129092)
 (vector-slots 8 2341344 202175)
 (floats 8 1075 874)
 (intervals 56 26101 3923)
 (buffers 992 54))

-- 
Vennlig hilsen
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[Message part 2 (text/html, inline)]

Message #8 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: 59660 <at> debbugs.gnu.org, Theodor Thornhill <theo <at> thornhill.no>,
 Yuan Fu <casouri <at> gmail.com>
Subject: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore method-name
 fontification
Date: Mon, 28 Nov 2022 16:09:01 +0100

[Message part 1 (text/plain, inline)]

Attached is a simple patch which addresses this font-lock issue and 
makes typescript-ts-mode much better to use.

-- 
Kind regards
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[Message part 2 (text/html, inline)]

[0004-lisp-progmodes-typescript-ts-mode.el-restore-method-.patch (text/x-patch, attachment)]

Message #11 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: 59660 <at> debbugs.gnu.org, Theodor Thornhill <theo <at> thornhill.no>,
 Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore
 method-name fontification
Date: Mon, 28 Nov 2022 17:44:14 +0100

[Message part 1 (text/plain, inline)]

This additional patch further improves fontification by fontifying 
function-arguments as variables too.

--

Kind regards
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
On 28.11.2022 16:09, Jostein Kjønigsen wrote:
>
> Attached is a simple patch which addresses this font-lock issue and 
> makes typescript-ts-mode much better to use.
>
> -- 
> Kind regards
> *Jostein Kjønigsen*
>
> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
> https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[Message part 2 (text/html, inline)]

[0005-lisp-progmodes-typescript-ts-mode.el-Fontify-functio.patch (text/x-patch, attachment)]

Message #14 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: 59660 <at> debbugs.gnu.org, Theodor Thornhill <theo <at> thornhill.no>,
 Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore
 method-name fontification
Date: Mon, 28 Nov 2022 17:53:45 +0100

[Message part 1 (text/plain, inline)]

And another improvement.

--
Kind regards
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
On 28.11.2022 17:44, Jostein Kjønigsen wrote:
>
> This additional patch further improves fontification by fontifying 
> function-arguments as variables too.
>
> --
>
> Kind regards
> *Jostein Kjønigsen*
>
> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
> https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
> On 28.11.2022 16:09, Jostein Kjønigsen wrote:
>>
>> Attached is a simple patch which addresses this font-lock issue and 
>> makes typescript-ts-mode much better to use.
>>
>> -- 
>> Kind regards
>> *Jostein Kjønigsen*
>>
>> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>> https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[Message part 2 (text/html, inline)]

[0006-lisp-progmodes-typescript-ts-mode.el-Fontify-all-typ.patch (text/x-patch, attachment)]

Message #17 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: 59660 <at> debbugs.gnu.org, Theodor Thornhill <theo <at> thornhill.no>,
 Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore
 method-name fontification
Date: Mon, 28 Nov 2022 18:08:01 +0100

[Message part 1 (text/plain, inline)]

All fixes and one more squashed into the attached patch :)

Vennlig hilsen
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
On 28.11.2022 17:53, Jostein Kjønigsen wrote:
>
> And another improvement.
>
> --
> Kind regards
> *Jostein Kjønigsen*
>
> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
> https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
> On 28.11.2022 17:44, Jostein Kjønigsen wrote:
>>
>> This additional patch further improves fontification by fontifying 
>> function-arguments as variables too.
>>
>> --
>>
>> Kind regards
>> *Jostein Kjønigsen*
>>
>> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>> https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
>> On 28.11.2022 16:09, Jostein Kjønigsen wrote:
>>>
>>> Attached is a simple patch which addresses this font-lock issue and 
>>> makes typescript-ts-mode much better to use.
>>>
>>> -- 
>>> Kind regards
>>> *Jostein Kjønigsen*
>>>
>>> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>>> https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[Message part 2 (text/html, inline)]

[0004-lisp-progmodes-typescript-ts-mode.el-fontification-i.patch (text/x-patch, attachment)]

Message #20 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jostein <at> kjonigsen.net
Cc: 59660 <at> debbugs.gnu.org, casouri <at> gmail.com, theo <at> thornhill.no
Subject: Re: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore
 method-name fontification
Date: Mon, 28 Nov 2022 19:27:00 +0200

> Date: Mon, 28 Nov 2022 18:08:01 +0100
> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
> 
> All fixes and one more squashed into the attached patch :)

Which "all", please?  AFAICT, you also posted a patch in another bug number,
so is that also included?

Message #23 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Eli Zaretskii <eliz <at> gnu.org>, jostein <at> kjonigsen.net
Cc: 59660 <at> debbugs.gnu.org, casouri <at> gmail.com, theo <at> thornhill.no
Subject: Re: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore
 method-name fontification
Date: Mon, 28 Nov 2022 19:00:53 +0100

[Message part 1 (text/plain, inline)]

On 28.11.2022 18:27, Eli Zaretskii wrote:
>> Date: Mon, 28 Nov 2022 18:08:01 +0100
>> From: Jostein Kjønigsen<jostein <at> secure.kjonigsen.net>
>>
>> All fixes and one more squashed into the attached patch :)
> Which "all", please?  AFAICT, you also posted a patch in another bug number,
> so is that also included?

No.

I've registered separate bugs for csharp-ts-mode and typescript-ts-mode, 
since they are distinct major-modes with no code in common.

As such patches for C# has been sent to the C# bug.

And TypeScript patches have been sent to the TypeScript-bug.

As for "all", both csharp-ts-mode and typescript-ts-mode are "new" 
modes, and as such probably will have a few ripening issues once tested 
on real production code-bases and daily use (like I do now),

In name of brewity, I decided not to file a bug-report for every single 
fontification issue I found, but rather bulk up the fixes per mode on 
the respective bugs for those two languages. I hope this is OK.

As for TypeScript, IMO there still are a few minor issues wrt to how 
font-lock-variable-name-face gets applied (or not), but I don't have 
patches for those yet, and I would not consider them deal-breakers or 
release-killers :)

--
Kind regards*
Jostein Kjønigsen*

[Message part 2 (text/html, inline)]

Message #26 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: jostein <at> kjonigsen.net
Cc: 59660 <at> debbugs.gnu.org, Theodor Thornhill <theo <at> thornhill.no>
Subject: Re: bug#59660: lisp/progmodes/typescript-ts-mode.el: restore
 method-name fontification
Date: Mon, 28 Nov 2022 15:02:09 -0800

> On Nov 28, 2022, at 9:08 AM, Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> wrote:
> 
> All fixes and one more squashed into the attached patch :)
> 
> Vennlig hilsen
> Jostein Kjønigsen
> 
> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
> https://jostein.kjønigsen.no
> On 28.11.2022 17:53, Jostein Kjønigsen wrote:
>> And another improvement. 
>> 
>> --
>> Kind regards
>> Jostein Kjønigsen
>> 
>> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>> https://jostein.kjønigsen.no
>> On 28.11.2022 17:44, Jostein Kjønigsen wrote:
>>> This additional patch further improves fontification by fontifying function-arguments as variables too.
>>> 
>>> --
>>> 
>>> Kind regards
>>> Jostein Kjønigsen
>>> 
>>> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>>> https://jostein.kjønigsen.no
>>> On 28.11.2022 16:09, Jostein Kjønigsen wrote:
>>>> Attached is a simple patch which addresses this font-lock issue and makes typescript-ts-mode much better to use.
>>>> 
>>>> -- 
>>>> Kind regards
>>>> Jostein Kjønigsen
>>>> 
>>>> jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>>>> https://jostein.kjønigsen.no
> <0004-lisp-progmodes-typescript-ts-mode.el-fontification-i.patch>

Thanks! Applied with minor change to commit message.

Yuan

Message #29 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: jostein <at> kjonigsen.net
Cc: 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
Subject: Re: bug#59660: 29.0.50; typescript-ts-mode consistently fontifies
 method-names incorrectly
Date: Mon, 28 Nov 2022 15:06:30 -0800

> On Nov 28, 2022, at 7:04 AM, Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> wrote:
> 
> From: "Jostein Kjønigsen" <jostein <at> ThinkPad-T14s.mail-host-address-is-not-set>
> To: bug-gnu-emacs <at> gnu.org
> Subject: 29.0.50; typescript-ts-mode consistently fontifies method-names incorrectly
> --text follows this line--
> 
> --
> When I:
> 
> 1. create a typescript file (sample.ts)
> 2. create class and add some methods to it.
> 
> I observe that:
> 
> - method names are consistently fontified as font-lock-property-face.
> 
> I expected that:
> 
> - method names to be consistently fontified as font-luck-function-name-face.
> 
> This seems to be because of the "property_identifier" is used for
> functions in the treesitter-grammar, and this property is used
> indiscriminately to apply font-lock-property-face at the bottom of the
> major-mode grammar.
> 
> This "shadows" the former rules which correctly apply font-lock-function-name-face.
> 
> The best would obviously have a way to identify properties more accurately, to precent
> shadowing from taking place. Unfortunately I cannot see a realiable way to do that
> based on the current parse-tree.
> 
> Seeing as properties in Typescript are fairly rare while methods are very common,
> it seems appropriate to me to remove the property-specific fontification rules
> to restore the method-fontification rules.
> 
> I got a patch for this ready, if people are willing to accept it.

Maybe you can remove the :override flag for the property_identifier rule, so it doesn’t shadow the function rule? Would that work better?

Yuan

Message #32 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 59660 <at> debbugs.gnu.org, jostein <at> kjonigsen.net, theo <at> thornhill.no
Subject: Re: bug#59660: 29.0.50;
 typescript-ts-mode consistently fontifies method-names incorrectly
Date: Tue, 29 Nov 2022 14:10:19 +0200

> Cc: 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Mon, 28 Nov 2022 15:06:30 -0800
> 
> > 1. create a typescript file (sample.ts)
> > 2. create class and add some methods to it.
> > 
> > I observe that:
> > 
> > - method names are consistently fontified as font-lock-property-face.
> > 
> > I expected that:
> > 
> > - method names to be consistently fontified as font-luck-function-name-face.
> > 
> > This seems to be because of the "property_identifier" is used for
> > functions in the treesitter-grammar, and this property is used
> > indiscriminately to apply font-lock-property-face at the bottom of the
> > major-mode grammar.
> > 
> > This "shadows" the former rules which correctly apply font-lock-function-name-face.
> > 
> > The best would obviously have a way to identify properties more accurately, to precent
> > shadowing from taking place. Unfortunately I cannot see a realiable way to do that
> > based on the current parse-tree.
> > 
> > Seeing as properties in Typescript are fairly rare while methods are very common,
> > it seems appropriate to me to remove the property-specific fontification rules
> > to restore the method-fontification rules.
> > 
> > I got a patch for this ready, if people are willing to accept it.
> 
> Maybe you can remove the :override flag for the property_identifier rule, so it doesn’t shadow the function rule? Would that work better?

This could be a personal preference, perhaps?  It isn't clear to me that
only one of the two is definitely correct.  So maybe we need a user option
for which one overrides which?

Message #35 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Eli Zaretskii <eliz <at> gnu.org>, Yuan Fu <casouri <at> gmail.com>
Cc: 59660 <at> debbugs.gnu.org, jostein <at> kjonigsen.net, theo <at> thornhill.no
Subject: Re: bug#59660: 29.0.50; typescript-ts-mode consistently fontifies
 method-names incorrectly
Date: Tue, 29 Nov 2022 14:58:59 +0100

Cc: 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Mon, 28 Nov 2022 15:06:30 -0800
>>
>> Maybe you can remove the :override flag for the property_identifier rule, so it doesn’t shadow the function rule? Would that work better?
I plan to test that as quickly as I time permits. If we can make both 
behaviours work, that would obviously be the best.
> This could be a personal preference, perhaps?  It isn't clear to me that
> only one of the two is definitely correct.  So maybe we need a user option
> for which one overrides which?

I honestly think this is reasonably clear.

"Properties" with getters and setters are at this point in time almost 
entirely unused in EcmaScript/TypeScript space. They are optional and 
have complicated syntax, and don't play well with JSON serialization 
(and thus can't be used in REST APIs).

You however cannot write a program without defining or invoking 
methods/functions. So not having method-fontification working correctly 
will literally impact **all** TypeScript-users.

--
Jostein

Message #38 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
Cc: 59660 <at> debbugs.gnu.org, casouri <at> gmail.com, jostein <at> kjonigsen.net,
 theo <at> thornhill.no
Subject: Re: bug#59660: 29.0.50; typescript-ts-mode consistently fontifies
 method-names incorrectly
Date: Tue, 29 Nov 2022 16:30:09 +0200

> Date: Tue, 29 Nov 2022 14:58:59 +0100
> Cc: jostein <at> kjonigsen.net, 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
> 
> > This could be a personal preference, perhaps?  It isn't clear to me that
> > only one of the two is definitely correct.  So maybe we need a user option
> > for which one overrides which?
> 
> I honestly think this is reasonably clear.
> 
> "Properties" with getters and setters are at this point in time almost 
> entirely unused in EcmaScript/TypeScript space. They are optional and 
> have complicated syntax, and don't play well with JSON serialization 
> (and thus can't be used in REST APIs).

So you are saying Typescript should not even support these features in the
tree-sitter based fontifications?  If so, I'm okay with that, but it sounds
like :override is the wrong tool for that, and we should simply remove
"properties" from the list of features for Typescript?

Message #41 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>, Eli
 Zaretskii <eliz <at> gnu.org>, Yuan Fu <casouri <at> gmail.com>
Cc: 59660 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#59660: 29.0.50; typescript-ts-mode consistently fontifies
 method-names incorrectly
Date: Tue, 29 Nov 2022 16:10:06 +0100

Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:

> Cc: 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
>>> From: Yuan Fu <casouri <at> gmail.com>
>>> Date: Mon, 28 Nov 2022 15:06:30 -0800
>>>
>>> Maybe you can remove the :override flag for the property_identifier rule, so it doesn’t shadow the function rule? Would that work better?
> I plan to test that as quickly as I time permits. If we can make both 
> behaviours work, that would obviously be the best.
>> This could be a personal preference, perhaps?  It isn't clear to me that
>> only one of the two is definitely correct.  So maybe we need a user option
>> for which one overrides which?
>
> I honestly think this is reasonably clear.
>
> "Properties" with getters and setters are at this point in time almost 
> entirely unused in EcmaScript/TypeScript space. They are optional and 
> have complicated syntax, and don't play well with JSON serialization 
> (and thus can't be used in REST APIs).
>
> You however cannot write a program without defining or invoking 
> methods/functions. So not having method-fontification working correctly 
> will literally impact **all** TypeScript-users.

Could you give me a code example, Jostein?  I'm just interested in a
repro, because I can't reproduce it with the typescript-ts-mode.

I tried using this from [0]:
```
class Greeter {
  greeting: string;

  constructor(message: string) {
    this.greeting = message;
  }

  greet() {
    return "Hello, " + this.greeting;
  }
}
```

On my end I get no property-face, just function-name-face.  Both on
constructor and greet.  I'm not saying I disagree with you, but what am
I missing?

Theo

[0]: https://www.typescriptlang.org/docs/handbook/classes.html

Message #44 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 59660 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, jostein <at> kjonigsen.net,
 Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#59660: 29.0.50;
 typescript-ts-mode consistently fontifies method-names incorrectly
Date: Tue, 29 Nov 2022 17:35:31 +0100

> On 29 Nov 2022, at 16:10, Theodor Thornhill <theo <at> thornhill.no> wrote:
> 
> Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:
> 
>> Cc: 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
>>>> From: Yuan Fu <casouri <at> gmail.com>
>>>> Date: Mon, 28 Nov 2022 15:06:30 -0800
>>>> 
>>>> Maybe you can remove the :override flag for the property_identifier rule, so it doesn’t shadow the function rule? Would that work better?
>> I plan to test that as quickly as I time permits. If we can make both 
>> behaviours work, that would obviously be the best.
>>> This could be a personal preference, perhaps?  It isn't clear to me that
>>> only one of the two is definitely correct.  So maybe we need a user option
>>> for which one overrides which?
>> 
>> I honestly think this is reasonably clear.
>> 
>> "Properties" with getters and setters are at this point in time almost 
>> entirely unused in EcmaScript/TypeScript space. They are optional and 
>> have complicated syntax, and don't play well with JSON serialization 
>> (and thus can't be used in REST APIs).
>> 
>> You however cannot write a program without defining or invoking 
>> methods/functions. So not having method-fontification working correctly 
>> will literally impact **all** TypeScript-users.
> 
> Could you give me a code example, Jostein?  I'm just interested in a
> repro, because I can't reproduce it with the typescript-ts-mode.
> 
> I tried using this from [0]:
> ```
> class Greeter {
>  greeting: string;
> 
>  constructor(message: string) {
>    this.greeting = message;
>  }
> 
>  greet() {
>    return "Hello, " + this.greeting;
>  }
> }
> ```
> 
> On my end I get no property-face, just function-name-face.  Both on
> constructor and greet.  I'm not saying I disagree with you, but what am
> I missing?
> 
> Theo
> 
> [0]: https://www.typescriptlang.org/docs/handbook/classes.html
> 

I’ve already submitted patches to fix this which Yuan has pushed. Try going a couple days back in time and try again ;) 

What remains to be decided is what we do with property-highlighting, and if we can find a way to enable that without colliding with function-names. 

—
Jostein Kjønigsen
https://jostein.kjønigsen.net

Message #47 received at 59660 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
Cc: 59660 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, jostein <at> kjonigsen.net,
 Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#59660: 29.0.50; typescript-ts-mode consistently fontifies method-names incorrectly
Date: Tue, 29 Nov 2022 17:39:44 +0100

On 29 November 2022 17:35:31 CET, "Jostein Kjønigsen" <jostein <at> secure.kjonigsen.net> wrote:
>
>
>> On 29 Nov 2022, at 16:10, Theodor Thornhill <theo <at> thornhill.no> wrote:
>> 
>> Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:
>> 
>>> Cc: 59660 <at> debbugs.gnu.org, theo <at> thornhill.no
>>>>> From: Yuan Fu <casouri <at> gmail.com>
>>>>> Date: Mon, 28 Nov 2022 15:06:30 -0800
>>>>> 
>>>>> Maybe you can remove the :override flag for the property_identifier rule, so it doesn’t shadow the function rule? Would that work better?
>>> I plan to test that as quickly as I time permits. If we can make both 
>>> behaviours work, that would obviously be the best.
>>>> This could be a personal preference, perhaps?  It isn't clear to me that
>>>> only one of the two is definitely correct.  So maybe we need a user option
>>>> for which one overrides which?
>>> 
>>> I honestly think this is reasonably clear.
>>> 
>>> "Properties" with getters and setters are at this point in time almost 
>>> entirely unused in EcmaScript/TypeScript space. They are optional and 
>>> have complicated syntax, and don't play well with JSON serialization 
>>> (and thus can't be used in REST APIs).
>>> 
>>> You however cannot write a program without defining or invoking 
>>> methods/functions. So not having method-fontification working correctly 
>>> will literally impact **all** TypeScript-users.
>> 
>> Could you give me a code example, Jostein?  I'm just interested in a
>> repro, because I can't reproduce it with the typescript-ts-mode.
>> 
>> I tried using this from [0]:
>> ```
>> class Greeter {
>>  greeting: string;
>> 
>>  constructor(message: string) {
>>    this.greeting = message;
>>  }
>> 
>>  greet() {
>>    return "Hello, " + this.greeting;
>>  }
>> }
>> ```
>> 
>> On my end I get no property-face, just function-name-face.  Both on
>> constructor and greet.  I'm not saying I disagree with you, but what am
>> I missing?
>> 
>> Theo
>> 
>> [0]: https://www.typescriptlang.org/docs/handbook/classes.html
>> 
>
>I’ve already submitted patches to fix this which Yuan has pushed. Try going a couple days back in time and try again ;) 
>
>What remains to be decided is what we do with property-highlighting, and if we can find a way to enable that without colliding with function-names. 
>
>—
>Jostein Kjønigsen
>https://jostein.kjønigsen.net

Ah, don't mind me, I'll hide in the corner :-)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.