bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

[-- Attachment #1: Type: text/plain, Size: 13504 bytes --]

When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have encountered so far:

When calling methods on objects, and that method accepts a generic type-argument. You typically see this in Startup.cs-like files in ASP.Net Core projects:

services.AddSomeExtensionWithoutTypeArguments();
services.AddSomeExtensionWithTypeArguments<MyType>();

In the above cases we see that fontification of "services" differs.

For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services is fontified using font-lock-function-name-face (incorrect).

In both cases I expected services to be fontified using font-lock-variable-name-face.

--
Jostein


In GNU Emacs 29.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.33, cairo version 1.16.0) of 2022-12-08 built on dev-jostein
Repository revision: 8fb2afe051168b2eac2bba0ee102039d0af90a05
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12201001
System Description: Ubuntu 22.04.1 LTS

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

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
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: C#

Minor modes in effect:
  electric-pair-mode: t
  editorconfig-mode: t
  treesit-explore-mode: t
  flycheck-mode: t
  highlight-symbol-mode: t
  which-function-mode: t
  nlinum-mode: t
  company-mode: t
  global-ede-mode: t
  ede-minor-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
  shell-dirtrack-mode: t
  helm--remap-mouse-mode: t
  async-bytecomp-package-mode: t
  delete-selection-mode: t
  global-auto-revert-mode: t
  server-mode: t
  global-hl-line-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  tooltip-mode: t
  global-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
  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-20221202.1727/transient hides /home/jostein/build/emacs/lisp/transient

Features:
(shadow sort flyspell ispell mail-extr emacsbug message yank-media
rfc822 mml mml-sec epa derived mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums face-remap semantic/lex-spp ede/emacs semantic/db
semantic/util-modes semantic/util semantic semantic/tag semantic/lex
semantic/fw mode-local bug-reference helm-command helm-elisp helm-eval
helm-info view vc-git diff-mode vc-dispatcher disp-table elec-pair
csharp-mode cc-langs cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs 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-astro 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 realgud realgud-zshdb realgud:zshdb-track-mode
realgud:zshdb-core realgud:zshdb-init realgud-trepan3k
realgud:trepan3k-track-mode realgud:trepan3k-core realgud:trepan3k-init
realgud-trepan2 realgud:trepan2-track-mode realgud:trepan2-core
realgud:trepan2-init realgud-trepanpl realgud:trepanpl-track-mode
realgud:trepanpl-core realgud:trepanpl-init realgud-trepanjs
realgud:trepanjs-track-mode realgud:trepanjs-core realgud:trepanjs-init
realgud-lang-js realgud-trepan realgud:trepan-track-mode
realgud:trepan-core realgud:trepan-init realgud-remake
realgud:remake-track-mode realgud:remake-core realgud:remake-init
realgud-rdebug realgud-rdebug-track-mode realgud-rdebug-core
realgud-rdebug-init realgud-lang-ruby realgud-perldb
realgud:perldb-track-mode realgud:perldb-core realgud:perldb-init
realgud-lang-perl realgud-pdb realgud:pdb-track-mode realgud:pdb-core
realgud:pdb-init realgud-lang-python python treesit realgud-kshdb
realgud:kshdb-track-mode realgud:kshdb-core realgud:kshdb-init
realgud-gub realgud:gub-track-mode realgud:gub-core realgud:gub-init
realgud-gdb realgud:gdb-track-mode realgud:gdb-init realgud:gdb-core
realgud-bashdb realgud:bashdb-track-mode realgud:bashdb-core
realgud:bashdb-init realgud-lang-posix-shell realgud:run
realgud-locals-mode realgud-breakpoint-mode realgud-backtrack-mode
realgud-track-mode realgud-backtrace-mode realgud-attach
realgud-lang-java realgud-track realgud-shortkey realgud-menu
realgud-eval realgud-cmds realgud-send realgud-window realgud-utils
eshell realgud-init realgud-file realgud-core realgud-reset
realgud-buffer-helper realgud-buffer-breakpoint realgud-buffer-backtrace
realgud-locals realgud-buffer-locals realgud-buffer-command
realgud-buffer-info realgud-lochist realgud-bp realgud-bp-image-data
realgud-lang esh-mode esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg
esh-module esh-groups esh-util cus-start realgud-loc
realgud-buffer-source realgud-key key realgud-follow realgud-fringe
realgud-helper loc-changes realgud-regexp realgud-custom load-relative
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 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-src ob-comint org-pcomplete org-list
org-footnote org-faces org-entities ob-emacs-lisp ob-core ob-eval
org-cycle org-table ol org-fold org-fold-core org-keys oc org-loaddefs
find-func cal-menu calendar cal-loaddefs org-version org-compat org-macs
ido-yes-or-no advice 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 shell
pcomplete 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 server 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 tree-sitter-indent-autoloads expand-region-autoloads
ido-yes-or-no-autoloads nodejs-repl-autoloads marmalade-client-autoloads
doom-modeline-autoloads elisp-slime-nav-autoloads
multiple-cursors-autoloads dracula-theme-autoloads
git-timemachine-autoloads crontab-mode-autoloads
highlight-symbol-autoloads ssh-config-mode-autoloads langtool-autoloads
dap-mode-autoloads lsp-treemacs-autoloads treemacs-autoloads
cfrs-autoloads posframe-autoloads gh-autoloads marshal-autoloads
logito-autoloads pcache-autoloads kv-autoloads toml-mode-autoloads
company-web-autoloads shrink-path-autoloads realgud-autoloads
realgud-recursive-autoloads load-relative-autoloads paredit-autoloads
bui-autoloads tree-sitter-langs-autoloads helpful-autoloads
elisp-refs-autoloads magit-autoloads magit-section-autoloads
git-commit-autoloads with-editor-autoloads flycheck-package-autoloads
package-lint-autoloads rust-mode-autoloads yaml-mode-autoloads
macrostep-autoloads lsp-docker-autoloads yaml-autoloads
lsp-mode-autoloads spinner-autoloads f-autoloads hydra-autoloads
nlinum-autoloads ht-autoloads loc-changes-autoloads transient-autoloads
compat-autoloads web-mode-autoloads undo-tree-autoloads queue-autoloads
ace-window-autoloads avy-autoloads bmx-mode-autoloads company-autoloads
test-simple-autoloads js2-mode-autoloads web-autoloads s-autoloads
ido-completing-read+-autoloads memoize-autoloads lv-autoloads
imenu-anywhere-autoloads helm-projectile-autoloads projectile-autoloads
helm-autoloads popup-autoloads helm-core-autoloads async-autoloads
tree-sitter-autoloads tsc-autoloads editorconfig-autoloads
json-mode-autoloads rx json-snatcher-autoloads yasnippet-autoloads
web-completion-data-autoloads cargo-autoloads markdown-mode-autoloads
pfuture-autoloads flycheck-autoloads pkg-info-autoloads epl-autoloads
info dash-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 592007 30876)
(symbols 48 54353 1)
(strings 32 189953 13159)
(string-bytes 1 6128946)
(vectors 16 108411)
(vector-slots 8 2026816 187502)
(floats 8 550 561)
(intervals 56 4720 24)
(buffers 992 25))


--
Vennlig hilsen
Jostein Kjønigsen

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.xn--kjnigsen-64a.no/>

[-- Attachment #2: Type: text/html, Size: 17713 bytes --]

bug#59897: Acknowledgement (29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.)

[Jostein Kjønigsen]

[-- Attachment #1.1: Type: text/plain, Size: 819 bytes --]

And here's the patch to fix it.


*Jostein Kjønigsen*
jostein.kjønigsen.no <https://jostein.kjønigsen.no>
jostein@kjonigsen.net - jostein@gmail.com
On 08.12.2022 10:30, GNU bug Tracking System wrote:
> Thank you for filing a new bug report with debbugs.gnu.org.
>
> This is an automatically generated reply to let you know your message
> has been received.
>
> Your message is being forwarded to the package maintainers and other
> interested parties for their attention; they will reply in due course.
>
> Your message has been sent to the package maintainer(s):
>   bug-gnu-emacs@gnu.org
>
> If you wish to submit further information on this problem, please
> send it to59897@debbugs.gnu.org.
>
> Please do not send mail tohelp-debbugs@gnu.org  unless you wish
> to report a problem with the Bug-tracking system.
>

[-- Attachment #1.2: Type: text/html, Size: 1676 bytes --]

[-- Attachment #2: 0001-lisp-progmodes-csharp-mode.el-improve-fontification-.patch --]
[-- Type: text/x-patch, Size: 1508 bytes --]

From ab73c1ea068538e79b06012b912c02b0e961b841 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jostein=20Kj=C3=B8nigsen?= <jostein@kjonigsen.net>
Date: Thu, 8 Dec 2022 11:07:15 +0100
Subject: [PATCH] lisp/progmodes/csharp-mode.el: improve fontification in
 csharp-ts-mode

Fixes highlighting of generic methods like the one below:

instance.MethodWithTypeArguments<Type>(...);
InClassMethodWithTypeArguments<Type>(...);
---
 lisp/progmodes/csharp-mode.el | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 82e3bc0d541..12e45272551 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -791,7 +791,15 @@ csharp-ts-mode--font-lock-settings
      (invocation_expression
       (identifier) @font-lock-function-name-face)
      (invocation_expression
-      (member_access_expression (identifier) @font-lock-function-name-face))
+      (member_access_expression
+       expression: (identifier) @font-lock-variable-name-face))
+     (invocation_expression
+      function: [(generic_name (identifier)) @font-lock-function-name-face
+                 (generic_name (type_argument_list
+                                ["<"] @font-lock-bracket-face
+                                (identifier) @font-lock-type-face
+                                [">"] @font-lock-bracket-face)
+                               )])
 
      (catch_declaration
       ((identifier) @font-lock-type-face))
-- 
2.34.1

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]

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

> When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
> encountered so far:
>
> When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
> Startup.cs-like files in ASP.Net Core projects:
>
> services.AddSomeExtensionWithoutTypeArguments();
> services.AddSomeExtensionWithTypeArguments<MyType>();
>
> In the above cases we see that fontification of "services" differs.
>
> For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
> is fontified using font-lock-function-name-face (incorrect).
>
> In both cases I expected services to be fontified using font-lock-variable-name-face.
>

Can you test this patch, Jostein, and if you're happy, please install,
Yuan :-)

BTW, I think the ruleset is getting pretty gnarly in csharp-mode, so
maybe we should consider slimming it down a little (without losing
granularity) for perf reasons down the line!

Theo



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-expression-for-generic_name-in-csharp-ts-mode.patch --]
[-- Type: text/x-diff, Size: 1441 bytes --]

From 34fa2bc67ddffca65f219644f055f75a2b073aba Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Thu, 8 Dec 2022 11:08:28 +0100
Subject: [PATCH] Add expression for generic_name in csharp-ts-mode

Given the below example, we want 'services' to be font-locked in
'font-lock-variable-name-face' in all cases.  Previously this only
worked in the first case, and the other was font-locked as
'font-lock-function-name-face'.

namespace Foo {
    void Foo() {
        services.Add();
        services.Add<MyType>();
    }
}

* lisp/progmodes/csharp-mode.el (csharp-ts-mode--font-lock-settings):
Add new query that makes the mentioned example work.

(bug#59897)
---
 lisp/progmodes/csharp-mode.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 82e3bc0d54..bedf3f4342 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -784,6 +784,10 @@ csharp-ts-mode--font-lock-settings
      (invocation_expression
       (member_access_expression
        (generic_name (identifier) @font-lock-function-name-face)))
+     (invocation_expression
+      (member_access_expression
+       expression: (identifier) @font-lock-variable-name-face
+       name: (generic_name (type_argument_list (identifier)))))
      (invocation_expression
       (member_access_expression
        ((identifier) @font-lock-variable-name-face
-- 
2.34.1

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

On 08.12.2022 11:12, Theodor Thornhill wrote:
> Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:
>
>> When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
>> encountered so far:
>>
>> When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
>> Startup.cs-like files in ASP.Net Core projects:
>>
>> services.AddSomeExtensionWithoutTypeArguments();
>> services.AddSomeExtensionWithTypeArguments<MyType>();
>>
>> In the above cases we see that fontification of "services" differs.
>>
>> For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
>> is fontified using font-lock-function-name-face (incorrect).
>>
>> In both cases I expected services to be fontified using font-lock-variable-name-face.
>>
> Can you test this patch, Jostein, and if you're happy, please install,
> Yuan :-)

I beat you by 3 minutes, but I'll be a gentleman and test none the less :D

You test mine, and we can see which one we prefer?
> BTW, I think the ruleset is getting pretty gnarly in csharp-mode, so
> maybe we should consider slimming it down a little (without losing
> granularity) for perf reasons down the line!
>
> Theo

It's getting somewhat intricate, agreed. It's IMO not entirely obvious 
what the different rules are for or what cases they are meant to support.

In that regard, I miss our old test-suite from github's csharp-mode. It 
was a nice safety-net when making changes like this.

--
Jostein

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> On 08.12.2022 11:12, Theodor Thornhill wrote:
>> Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:
>>
>>> When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
>>> encountered so far:
>>>
>>> When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
>>> Startup.cs-like files in ASP.Net Core projects:
>>>
>>> services.AddSomeExtensionWithoutTypeArguments();
>>> services.AddSomeExtensionWithTypeArguments<MyType>();
>>>
>>> In the above cases we see that fontification of "services" differs.
>>>
>>> For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
>>> is fontified using font-lock-function-name-face (incorrect).
>>>
>>> In both cases I expected services to be fontified using font-lock-variable-name-face.
>>>
>> Can you test this patch, Jostein, and if you're happy, please install,
>> Yuan :-)
>
> I beat you by 3 minutes, but I'll be a gentleman and test none the less :D
>
> You test mine, and we can see which one we prefer?

Sure!  Both seems to work from what I can tell :-)  I'll let you be the
judge!

Theo

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

[-- Attachment #1: Type: text/plain, Size: 2292 bytes --]

On 08.12.2022 12:12, Theodor Thornhill wrote:
> Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
>
>> On 08.12.2022 11:12, Theodor Thornhill wrote:
>>> Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
>>>
>>>> When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
>>>> encountered so far:
>>>>
>>>> When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
>>>> Startup.cs-like files in ASP.Net Core projects:
>>>>
>>>> services.AddSomeExtensionWithoutTypeArguments();
>>>> services.AddSomeExtensionWithTypeArguments<MyType>();
>>>>
>>>> In the above cases we see that fontification of "services" differs.
>>>>
>>>> For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
>>>> is fontified using font-lock-function-name-face (incorrect).
>>>>
>>>> In both cases I expected services to be fontified using font-lock-variable-name-face.
>>>>
>>> Can you test this patch, Jostein, and if you're happy, please install,
>>> Yuan :-)
>> I beat you by 3 minutes, but I'll be a gentleman and test none the less :D
>>
>> You test mine, and we can see which one we prefer?
> Sure!  Both seems to work from what I can tell :-)  I'll let you be the
> judge!
>
> Theo

Your patch solves the issue described in the bug, but does not handle 
another fontification error I discovered while testing my patch:

SimpleGenericMethod<Type>(params);

In the above example SimpleGenericMethod is fontified using 
font-lock-type-face instead of font-lock-function-name-face. My patch 
fixes that case as well.

As for which patch to choose:

  *  From an objective perspective, the way I understand the code, your
    patch overrides an existing fontification to apply variable-name
    instead.
  * My patch however changes some selectors to be more specific
    selectors to avoid fontifying the variable-identifier, and also
    creates a new, highly-specific selector to fontify the variable-name
    aspect as well.

 From a performance perspective, I would assume the latter approach is 
more performant, but I don't know enough tree-sitter internals to say 
that with 100% confidence.

Does anyone else know?

--
Jostein

[-- Attachment #2: Type: text/html, Size: 3336 bytes --]

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]

On 08.12.2022 12:31, Jostein Kjønigsen wrote:
> On 08.12.2022 12:12, Theodor Thornhill wrote:
>> Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
>>
>>> On 08.12.2022 11:12, Theodor Thornhill wrote:
>>>> Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
>>>>
>>>>> When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
>>>>> encountered so far:
>>>>>
>>>>> When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
>>>>> Startup.cs-like files in ASP.Net Core projects:
>>>>>
>>>>> services.AddSomeExtensionWithoutTypeArguments();
>>>>> services.AddSomeExtensionWithTypeArguments<MyType>();
>>>>>
>>>>> In the above cases we see that fontification of "services" differs.
>>>>>
>>>>> For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
>>>>> is fontified using font-lock-function-name-face (incorrect).
>>>>>
>>>>> In both cases I expected services to be fontified using font-lock-variable-name-face.
>>>>>
>>>> Can you test this patch, Jostein, and if you're happy, please install,
>>>> Yuan :-)
>>> I beat you by 3 minutes, but I'll be a gentleman and test none the less :D
>>>
>>> You test mine, and we can see which one we prefer?
>> Sure!  Both seems to work from what I can tell :-)  I'll let you be the
>> judge!
>>
>> Theo
>
> Your patch solves the issue described in the bug, but does not handle 
> another fontification error I discovered while testing my patch:
>
> SimpleGenericMethod<Type>(params);
>
> In the above example SimpleGenericMethod is fontified using 
> font-lock-type-face instead of font-lock-function-name-face. My patch 
> fixes that case as well.
>
> As for which patch to choose:
>
>   * From an objective perspective, the way I understand the code, your
>     patch overrides an existing fontification to apply variable-name
>     instead.
>   * My patch however changes some selectors to be more specific
>     selectors to avoid fontifying the variable-identifier, and also
>     creates a new, highly-specific selector to fontify the
>     variable-name aspect as well.
>
> From a performance perspective, I would assume the latter approach is 
> more performant, but I don't know enough tree-sitter internals to say 
> that with 100% confidence.
>
> Does anyone else know?
>
> --
> Jostein
>
Hey guys.

This patch seems to have been left out, or slightly forgotten.

Yuan, I think in this case we are going to prefer my patch over Theo's 
since it fixes 2 issues, instead of just 1.

Could you install this? :)

--
Jostein

[-- Attachment #2: Type: text/html, Size: 4055 bytes --]

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Yuan Fu]

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

> On 08.12.2022 11:12, Theodor Thornhill wrote:
>> Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:
>>
>>> When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
>>> encountered so far:
>>>
>>> When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
>>> Startup.cs-like files in ASP.Net Core projects:
>>>
>>> services.AddSomeExtensionWithoutTypeArguments();
>>> services.AddSomeExtensionWithTypeArguments<MyType>();
>>>
>>> In the above cases we see that fontification of "services" differs.
>>>
>>> For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
>>> is fontified using font-lock-function-name-face (incorrect).
>>>
>>> In both cases I expected services to be fontified using font-lock-variable-name-face.
>>>
>> Can you test this patch, Jostein, and if you're happy, please install,
>> Yuan :-)
>
> I beat you by 3 minutes, but I'll be a gentleman and test none the less :D
>
> You test mine, and we can see which one we prefer?
>> BTW, I think the ruleset is getting pretty gnarly in csharp-mode, so
>> maybe we should consider slimming it down a little (without losing
>> granularity) for perf reasons down the line!
>>
>> Theo
>
> It's getting somewhat intricate, agreed. It's IMO not entirely obvious
> what the different rules are for or what cases they are meant to
> support.

We had a discussion on emacs-devel that aims to standardize and define
feature names, here is the list so far:

Basic tokens:

delimiter       ,.;      (delimit things)
operator        == != || (produces a value)
bracket         []{}()
misc-punctuation

constant        true, false, null
number
keyword
comment         (includes doc-comments)
string          (includes chars and docstrings)
string-interpolation    f"text {variable}"
escape-sequence         "\n\t\\"
function                every function identifier
variable                every variable identifier
type                    every type identifier
property                a.b  <--- highlight b
key                     { a: b, c: d } <--- highlight a, c
error                   highlight parse error

Abstract features:

assignment: the LHS of an assignment (thing being assigned to), eg:

a = b    <--- highlight a
a.b = c  <--- highlight b
a[1] = d <--- highlight a

definition: the thing being defined, eg:

int a(int b) { <--- highlight a
return 0
}

int a;  <-- highlight a

struct a { <--- highlight a
int b;   <--- highlight b
}

>
> In that regard, I miss our old test-suite from github's csharp-mode.
> It was a nice safety-net when making changes like this.

A test suite is most welcome. I think we can even come up with a general
framework for testing fontification, indent, defun movement, imenu, etc.
To generate a test you would just need to open a file, verify everything
is expected, and run a command which records the current fontification
and indentation, etc, to a file. Then a test suite will just use that
file. I wonder if csharp-mode had something like this?

Yuan

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Yuan Fu]

> Your patch solves the issue described in the bug, but does not handle another fontification error I discovered while
> testing my patch:
>
> SimpleGenericMethod<Type>(params);
>
> In the above example SimpleGenericMethod is fontified using font-lock-type-face instead of
> font-lock-function-name-face. My patch fixes that case as well.
>
> As for which patch to choose:
>
> * From an objective perspective, the way I understand the code, your patch overrides an existing fontification to apply
>  variable-name instead.
> * My patch however changes some selectors to be more specific selectors to avoid fontifying the variable-identifier, and
>  also creates a new, highly-specific selector to fontify the variable-name aspect as well.
>
> From a performance perspective, I would assume the latter approach is more performant, but I don't know enough
> tree-sitter internals to say that with 100% confidence.
>
> Does anyone else know?

I don’t think the two approach would have significant difference. But I think
more specific pattern is definitely better in terms of code organization.

Yuan

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi!

>>
>> In that regard, I miss our old test-suite from github's csharp-mode.
>> It was a nice safety-net when making changes like this.
>
> A test suite is most welcome. I think we can even come up with a general
> framework for testing fontification, indent, defun movement, imenu, etc.
> To generate a test you would just need to open a file, verify everything
> is expected, and run a command which records the current fontification
> and indentation, etc, to a file. Then a test suite will just use that
> file. I wonder if csharp-mode had something like this?
>

Yeah we had some trickery to check the font-locking and indentation, but
it relies on external packages IIRC.

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Hey guys.
>
> This patch seems to have been left out, or slightly forgotten.
>
> Yuan, I think in this case we are going to prefer my patch over Theo's 
> since it fixes 2 issues, instead of just 1.
>
> Could you install this? :)
>

I concur :-)

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

On 09.12.2022 22:12, Theodor Thornhill wrote:
>> Hey guys.
>>
>> This patch seems to have been left out, or slightly forgotten.
>>
>> Yuan, I think in this case we are going to prefer my patch over Theo's
>> since it fixes 2 issues, instead of just 1.
>>
>> Could you install this? :)
>>
> I concur :-)

Looking at my local emacs-29 branch it seems also this patch been 
"forgotten" and not been applied.

Do you have time to get this installed, Yuan?

--
Jostein

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Yuan Fu]

> On Dec 11, 2022, at 11:58 PM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
> 
> On 09.12.2022 22:12, Theodor Thornhill wrote:
>>> Hey guys.
>>> 
>>> This patch seems to have been left out, or slightly forgotten.
>>> 
>>> Yuan, I think in this case we are going to prefer my patch over Theo's
>>> since it fixes 2 issues, instead of just 1.
>>> 
>>> Could you install this? :)
>>> 
>> I concur :-)
> 
> Looking at my local emacs-29 branch it seems also this patch been "forgotten" and not been applied.
> 
> Do you have time to get this installed, Yuan?

Yes, of course, my apologies :-) Also, beautiful commit message, Jostein, I love it :-)

Yuan

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

On 12.12.2022 23:19, Yuan Fu wrote:
> On 09.12.2022 22:12, Theodor Thornhill wrote:
>>>> Hey guys.
>>>>
>>>> This patch seems to have been left out, or slightly forgotten.
>>>>
>>>> Yuan, I think in this case we are going to prefer my patch over Theo's
>>>> since it fixes 2 issues, instead of just 1.
>>>>
>>>> Could you install this? :)
>>>>
>>> I concur :-)
>> Looking at my local emacs-29 branch it seems also this patch been "forgotten" and not been applied.
>>
>> Do you have time to get this installed, Yuan?
> Yes, of course, my apologies :-) Also, beautiful commit message, Jostein, I love it :-)
>
> Yuan

I see bug marked as done, but I don't see this pushed to the emacs-29 
branch.

Are you sure this has been installed Yuan?

--
Jostein

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Yuan Fu]

> On Dec 14, 2022, at 10:49 AM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
> 
> On 12.12.2022 23:19, Yuan Fu wrote:
>> On 09.12.2022 22:12, Theodor Thornhill wrote:
>>>>> Hey guys.
>>>>> 
>>>>> This patch seems to have been left out, or slightly forgotten.
>>>>> 
>>>>> Yuan, I think in this case we are going to prefer my patch over Theo's
>>>>> since it fixes 2 issues, instead of just 1.
>>>>> 
>>>>> Could you install this? :)
>>>>> 
>>>> I concur :-)
>>> Looking at my local emacs-29 branch it seems also this patch been "forgotten" and not been applied.
>>> 
>>> Do you have time to get this installed, Yuan?
>> Yes, of course, my apologies :-) Also, beautiful commit message, Jostein, I love it :-)
>> 
>> Yuan
> 
> I see bug marked as done, but I don't see this pushed to the emacs-29 branch.
> 
> Are you sure this has been installed Yuan?
> 
> --
> Jostein

Indeed, I forgot to push my local branch. I just did that. Now you should see it, sorry for the inconvenience :-)

Yuan

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Jostein Kjønigsen]

[-- Attachment #1.1: Type: text/html, Size: 8632 bytes --]

[-- Attachment #1.2: favicon.ico --]
[-- Type: image/x-icon, Size: 3290 bytes --]

bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.

[Yuan Fu]

> On Dec 14, 2022, at 1:45 PM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
> 
> Hey Yuan. 
> 
> From what I can see you pushed Teos patch and not mine?
> 
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-29&id=647b6a8099f414e5a7b162efd3658e174064dfe7emacs.git - Emacs source repository
> savannah.gnu.org
> <favicon.ico>
> 
> 
> In the email discussion earlier we unanymously decided on that for technical reasons my patch was more appropriate for this particular bug, and it also fixed another fontification bug. 
> 
> What do we do now? Should we revert this commit and apply mine instead?
> 
> Sorry for all the “noise”, but this gets hard to follow up for all of us when latency between input and output gets too long, and then we end up writing emails like this, instead of writing good code to improve emacs and our new major-modes. 
> 
> If this could get cleaned up, installed properly, confirmed -and- pushed at the same time, it would be easier for me to verify that the right patch was put in place, and then I wouldn’t nag you for weeks end for what is really just 4 lines of code ;)

Ahhhhh sorry! I reverted the old commit and applied yours, you should now see it. I’ll try to process these patches more promptly in the future.

Yuan