GNU bug report logs - #61104
29.0.60; typescript-ts-mode does not provide compilation-mode support

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 61104 in the body.
You can then email your comments to 61104 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>
Cc: Yuan Fu <casouri <at> gmail.com>, Theodor Thornhill <theo <at> thornhill.no>
Subject: 29.0.60; typescript-ts-mode does not provide compilation-mode support
Date: Fri, 27 Jan 2023 21:14:30 +0100

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

When compiling a TypeScript project using tsc (or other tooling) from Emacs,
compilation errors and warnings are not highlighted by compilation-mode.

Consider the following output:

src/resources/document.ts:140:22 - error TS2362: The left-hand side of 
an arithmetic operation must be of type 'any', 'number', 'bigint' or an 
enum type.

140       return `File-${new Date() * 1}${ext}`;
                         ~~~~~~~~~~

This output should cause compilation-mode to highlight the error and 
provide code-navigation.

I know we explicitly added support for this to typescript.el back in the 
days, but I'm not
sure what the "right" thing to do is now that typescrip-ts-mode is a 
first class Emacs citizen.

Should we add compilation-mode patterns directly to the major-mode, or 
should we provide
patches to compile.el instead?

Does anyone have any strong opinion or guidance here?

--
Jostein


In GNU Emacs 29.0.60 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.36, cairo version 1.17.6) of 2023-01-26 built on thinkpad-t14s
Repository revision: f8c95d1a7681e861fc22d2a040cda0ddfe23eff4
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12201007
System Description: Arch Linux

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

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

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

Major mode: JSON

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  electric-pair-mode: t
  highlight-symbol-mode: t
  flycheck-mode: t
  editorconfig-mode: t
  company-mode: t
  which-function-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
  yas-global-mode: t
  yas-minor-mode: t
  global-nlinum-mode: t
  nlinum-mode: t
  ido-yes-or-no-mode: t
  override-global-mode: t
  server-mode: t
  global-hl-line-mode: t
  pixel-scroll-precision-mode: t
  doom-modeline-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
  hs-minor-mode: t

Load-path shadows:
/home/jostein/.emacs.d/elpa/transient-20230107.1528/transient hides 
/home/jostein/build/emacs/lisp/transient

Features:
(shadow sort mail-extr emacsbug help-fns radix-tree cl-print
json-ts-mode dired-aux textsec uni-scripts idna-mapping ucs-normalize
uni-confusable textsec-check gnutls network-stream url-http url-gw nsm
url-cache url-auth goto-addr misearch multi-isearch flyspell ispell
magit-extras 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 diff git-commit log-edit message sendmail
yank-media rfc822 mml mml-sec epa derived epg rfc6068 epg-config
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums 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 helm-command
helm-elisp helm-eval edebug helm-info vc-git diff-mode vc-dispatcher
disp-table bug-reference winner ffap tramp-archive tramp-gvfs
tramp-cache time-stamp zeroconf dbus face-remap executable markdown-mode
color add-log elec-pair typescript-ts-mode js c-ts-common cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs treesit vc-svn ido-completing-read+ memoize minibuf-eldef
elisp-slime-nav paredit highlight-symbol flycheck editorconfig
editorconfig-core editorconfig-core-handle editorconfig-fnmatch
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 eglot external-completion array
jsonrpc ert ewoc debug backtrace flymake-proc flymake warnings
which-func hideshow eww url-queue thingatpt shr pixel-fill kinsoku
url-file svg xml dom puny mm-url gnus nnheader gnus-util mail-utils
range mm-util mail-prsvr helm-imenu 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 wid-edit files-x tramp-compat
shell parse-time iso8601 ls-lisp helm-buffers helm-occur helm-tags
helm-locate helm-grep helm-regexp helm-utils helm-help helm-types helm
helm-global-bindings helm-easymenu helm-core async-bytecomp helm-source
helm-multi-match helm-lib async pcase imenu ob-plantuml org ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
pcomplete org-list org-footnote org-faces org-entities time-date
noutline outline icons 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 format-spec delsel
autorevert filenotify yasnippet nlinum linum ido-yes-or-no advice ido
edmacro kmacro use-package-bind-key bind-key easy-mmode xref project
server hl-line pixel-scroll cua-base compile-eslint compile
text-property-search comint ansi-osc ansi-color ring 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
rx f f-shortdoc s dash compat dracula-theme cl-extra help-mode
use-package-ensure use-package-core finder-inf yasnippet-autoloads
ido-yes-or-no-autoloads elisp-slime-nav-autoloads cmake-mode-autoloads
flycheck-autoloads pkg-info-autoloads magit-autoloads
all-the-icons-autoloads crontab-mode-autoloads powershell-autoloads
doom-modeline-autoloads undo-tree-autoloads rust-mode-autoloads
magit-section-autoloads paredit-autoloads dracula-theme-autoloads
cargo-autoloads yaml-mode-autoloads helm-autoloads popup-autoloads
queue-autoloads nlinum-autoloads bmx-mode-autoloads company-autoloads
git-commit-autoloads multiple-cursors-autoloads dap-mode-autoloads
lsp-treemacs-autoloads treemacs-autoloads cfrs-autoloads
posframe-autoloads hydra-autoloads pfuture-autoloads
ace-window-autoloads avy-autoloads bui-autoloads transient-autoloads
ido-completing-read+-autoloads memoize-autoloads with-editor-autoloads
compat-autoloads epl-autoloads lsp-docker-autoloads yaml-autoloads
highlight-symbol-autoloads expand-region-autoloads lsp-mode-autoloads
lv-autoloads markdown-mode-autoloads spinner-autoloads ht-autoloads
shrink-path-autoloads f-autoloads dash-autoloads s-autoloads info
editorconfig-autoloads helm-core-autoloads async-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 903185 104000)
 (symbols 48 46499 0)
 (strings 32 231885 9884)
 (string-bytes 1 6411082)
 (vectors 16 124883)
 (vector-slots 8 2338056 282804)
 (floats 8 984 1046)
 (intervals 56 17029 3032)
 (buffers 984 62))

-- 
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 61104 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org, theo <at> thornhill.no
Subject: Re: bug#61104: 29.0.60;
 typescript-ts-mode does not provide compilation-mode support
Date: Fri, 27 Jan 2023 22:30:02 +0200

> Cc: Yuan Fu <casouri <at> gmail.com>, Theodor Thornhill <theo <at> thornhill.no>
> Date: Fri, 27 Jan 2023 21:14:30 +0100
> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
> 
> When compiling a TypeScript project using tsc (or other tooling) from Emacs,
> compilation errors and warnings are not highlighted by compilation-mode.
> 
> Consider the following output:
> 
> src/resources/document.ts:140:22 - error TS2362: The left-hand side of an arithmetic operation must be of
> type 'any', 'number', 'bigint' or an enum type.
> 
> 140       return `File-${new Date() * 1}${ext}`;
>                          ~~~~~~~~~~
> 
> This output should cause compilation-mode to highlight the error and provide code-navigation.
> 
> I know we explicitly added support for this to typescript.el back in the days, but I'm not
> sure what the "right" thing to do is now that typescrip-ts-mode is a first class Emacs citizen.
> 
> Should we add compilation-mode patterns directly to the major-mode, or should we provide
> patches to compile.el instead?

Why isn't this part of compilation-error-regexp-alist-alist?

Message #11 received at 61104 <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: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org, theo <at> thornhill.no
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Fri, 27 Jan 2023 21:52:24 +0100

On 1/27/23 21:30, Eli Zaretskii wrote:
>
> Why isn't this part of compilation-error-regexp-alist-alist?

That is obviously what I think we should do.
We already have good and tested matcher expressions for this.

I was just uncertain about what the conventional way of adding those 
expressions to Emacs:

- adding it directly in the relevant major-mode's source-file, to 
improve code-locality?
- adding it in compile.el, to improve ability to oversee all 
expressions, and be able to optimize those?

I see csharp-mode.el has the expressions added directly there. Should I 
go about preparing patches doing
the same for typescript-ts-mode too?

--
Jostein

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org, theo <at> thornhill.no
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Sat, 28 Jan 2023 09:23:10 +0200

> Date: Fri, 27 Jan 2023 21:52:24 +0100
> Cc: 61104 <at> debbugs.gnu.org, casouri <at> gmail.com, theo <at> thornhill.no
> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
> 
> On 1/27/23 21:30, Eli Zaretskii wrote:
> >
> > Why isn't this part of compilation-error-regexp-alist-alist?
> 
> That is obviously what I think we should do.
> We already have good and tested matcher expressions for this.
> 
> I was just uncertain about what the conventional way of adding those 
> expressions to Emacs:
> 
> - adding it directly in the relevant major-mode's source-file, to 
> improve code-locality?

I don't think doing this in the major-mode file will improve locality,
because we have compilation-minor-mode, which should be able to do its
thing even if the relevant major mode is not yet loaded.

> - adding it in compile.el, to improve ability to oversee all 
> expressions, and be able to optimize those?
> 
> I see csharp-mode.el has the expressions added directly there. Should I 
> go about preparing patches doing
> the same for typescript-ts-mode too?

I don't see why not.

Message #17 received at 61104 <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: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org, theo <at> thornhill.no
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Sat, 28 Jan 2023 15:28:04 +0100

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

On 1/28/23 08:23, Eli Zaretskii wrote:
>
> I don't think doing this in the major-mode file will improve locality,
> because we have compilation-minor-mode, which should be able to do its
> thing even if the relevant major mode is not yet loaded.
Fair enough!
>
>> - adding it in compile.el, to improve ability to oversee all
>> expressions, and be able to optimize those?
>>
>> I see csharp-mode.el has the expressions added directly there. Should I
>> go about preparing patches doing
>> the same for typescript-ts-mode too?
> I don't see why not.

Ok. Attached is a patch which adds Typescript tsc-support to compile.el. 
Is this OK to install in emacs-29?

I also see in retrospect that my comment about csharp-mode above may 
been somewhat ambiguous and easy to misunderstand (and seemingly you did).

To be clear: csharp-mode includes the compilation-mode regexps in the 
major-mode, not in compile.el. Is this also something we should aim to 
fix for emacs-29, or should we leave that for master?

--
Jostein

[0001-Add-support-for-Typescript-compilation-to-compilatio.patch (text/x-patch, attachment)]

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

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org, theo <at> thornhill.no
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Thu, 2 Feb 2023 22:01:11 +0100

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

Any news on this one? Will this be merged? :)

Vennlig hilsen
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
On 1/28/23 15:28, Jostein Kjønigsen wrote:
>
> On 1/28/23 08:23, Eli Zaretskii wrote:
>>
>> I don't think doing this in the major-mode file will improve locality,
>> because we have compilation-minor-mode, which should be able to do its
>> thing even if the relevant major mode is not yet loaded.
> Fair enough!
>>
>>> - adding it in compile.el, to improve ability to oversee all
>>> expressions, and be able to optimize those?
>>>
>>> I see csharp-mode.el has the expressions added directly there. Should I
>>> go about preparing patches doing
>>> the same for typescript-ts-mode too?
>> I don't see why not.
>
> Ok. Attached is a patch which adds Typescript tsc-support to 
> compile.el. Is this OK to install in emacs-29?
>
> I also see in retrospect that my comment about csharp-mode above may 
> been somewhat ambiguous and easy to misunderstand (and seemingly you 
> did).
>
> To be clear: csharp-mode includes the compilation-mode regexps in the 
> major-mode, not in compile.el. Is this also something we should aim to 
> fix for emacs-29, or should we leave that for master?
>
> -- 
> Jostein

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

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: jostein <at> kjonigsen.net,
 Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support
Date: Fri, 03 Feb 2023 06:30:29 +0100

On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein <at> secure.kjonigsen.net> wrote:
>Any news on this one? Will this be merged? :)
>

My guess is that it should go on master. I'll look at it today - sorry it took some time :)

Theo


>Vennlig hilsen
>*Jostein Kjønigsen*
>
>jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
>https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
>On 1/28/23 15:28, Jostein Kjønigsen wrote:
>> 
>> On 1/28/23 08:23, Eli Zaretskii wrote:
>>> 
>>> I don't think doing this in the major-mode file will improve locality,
>>> because we have compilation-minor-mode, which should be able to do its
>>> thing even if the relevant major mode is not yet loaded.
>> Fair enough!
>>> 
>>>> - adding it in compile.el, to improve ability to oversee all
>>>> expressions, and be able to optimize those?
>>>> 
>>>> I see csharp-mode.el has the expressions added directly there. Should I
>>>> go about preparing patches doing
>>>> the same for typescript-ts-mode too?
>>> I don't see why not.
>> 
>> Ok. Attached is a patch which adds Typescript tsc-support to compile.el. Is this OK to install in emacs-29?
>> 
>> I also see in retrospect that my comment about csharp-mode above may been somewhat ambiguous and easy to misunderstand (and seemingly you did).
>> 
>> To be clear: csharp-mode includes the compilation-mode regexps in the major-mode, not in compile.el. Is this also something we should aim to fix for emacs-29, or should we leave that for master?
>> 
>> -- 
>> Jostein

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: jostein <at> secure.kjonigsen.net, casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 jostein <at> kjonigsen.net
Subject: Re: bug#61104: 29.0.60;
 typescript-ts-mode does not provide compilation-mode support
Date: Fri, 03 Feb 2023 09:06:22 +0200

> Date: Fri, 03 Feb 2023 06:30:29 +0100
> From: Theodor Thornhill <theo <at> thornhill.no>
> CC: 61104 <at> debbugs.gnu.org, casouri <at> gmail.com
> 
> On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein <at> secure.kjonigsen.net> wrote:
> >Any news on this one? Will this be merged? :)
> >
> 
> My guess is that it should go on master. I'll look at it today - sorry it took some time :)

I'm okay with installing this on emacs-29 if the patch looks good.
Typescript mode is new in Emacs 29, so it's okay to make this change
now.  But please be sure that the relevant tests still pass, i.e. that
this change doesn't break something else in compilation-mode.

Thanks.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jostein <at> secure.kjonigsen.net, casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 jostein <at> kjonigsen.net
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Fri, 03 Feb 2023 09:00:48 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Fri, 03 Feb 2023 06:30:29 +0100
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> CC: 61104 <at> debbugs.gnu.org, casouri <at> gmail.com
>> 
>> On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein <at> secure.kjonigsen.net> wrote:
>> >Any news on this one? Will this be merged? :)
>> >
>> 
>> My guess is that it should go on master. I'll look at it today - sorry it took some time :)
>
> I'm okay with installing this on emacs-29 if the patch looks good.
> Typescript mode is new in Emacs 29, so it's okay to make this change
> now.  But please be sure that the relevant tests still pass, i.e. that
> this change doesn't break something else in compilation-mode.
>
> Thanks.

Ok, I'll do that then.  I'll add this in addition in the next commit, if
that's ok.

Thanks, Jostein and Eli :)

Theo

diff --git a/test/lisp/progmodes/compile-tests.el b/test/lisp/progmodes/compile-tests.el
index 53dc7f2a13..22721563df 100644
--- a/test/lisp/progmodes/compile-tests.el
+++ b/test/lisp/progmodes/compile-tests.el
@@ -382,9 +382,13 @@ compile-tests--test-regexps-data
     ;; sun-ada
     (sun-ada "/home3/xdhar/rcds_rc/main.a, line 361, char 6:syntax error: \",\" inserted"
      1 6 361 "/home3/xdhar/rcds_rc/main.a")
+    (typescript-tsc-plain "/home/foo/greeter.ts(30,12): error TS2339: Property 'foo' does not exist."
+     1 12 30 "/home/foo/greeter.ts")
+    (typescript-tsc-pretty "src/resources/document.ts:140:22 - error TS2362: something."
+     1 22 140 "src/resources/document.ts")
     ;; 4bsd
     (edg-1 "/usr/src/foo/foo.c(8): warning: w may be used before set"
-     1 nil 8 "/usr/src/foo/foo.c")
+           1 nil 8 "/usr/src/foo/foo.c")
     (edg-1 "/usr/src/foo/foo.c(9): error: w is used before set"
      1 nil 9 "/usr/src/foo/foo.c")
     (4bsd "strcmp: variable # of args. llib-lc(359)  ::  /usr/src/foo/foo.c(8)"
@@ -495,7 +499,7 @@ compile-test-error-regexps
           (compilation-num-warnings-found 0)
           (compilation-num-infos-found 0))
       (mapc #'compile--test-error-line compile-tests--test-regexps-data)
-      (should (eq compilation-num-errors-found 98))
+      (should (eq compilation-num-errors-found 100))
       (should (eq compilation-num-warnings-found 35))
       (should (eq compilation-num-infos-found 28)))))
 

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jostein <at> kjonigsen.net, casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 jostein <at> secure.kjonigsen.net
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Sat, 04 Feb 2023 09:24:49 +0100

Theodor Thornhill <theo <at> thornhill.no> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Date: Fri, 03 Feb 2023 06:30:29 +0100
>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>> CC: 61104 <at> debbugs.gnu.org, casouri <at> gmail.com
>>> 
>>> On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein <at> secure.kjonigsen.net> wrote:
>>> >Any news on this one? Will this be merged? :)
>>> >
>>> 
>>> My guess is that it should go on master. I'll look at it today - sorry it took some time :)
>>
>> I'm okay with installing this on emacs-29 if the patch looks good.
>> Typescript mode is new in Emacs 29, so it's okay to make this change
>> now.  But please be sure that the relevant tests still pass, i.e. that
>> this change doesn't break something else in compilation-mode.
>>
>> Thanks.
>
> Ok, I'll do that then.  I'll add this in addition in the next commit, if
> that's ok.
>
> Thanks, Jostein and Eli :)

The changes are now installed on the emacs-29 branch, thanks!

Closing this :)

Theo

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

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 Theodor Thornhill <theo <at> thornhill.no>, Eli Zaretskii <eliz <at> gnu.org>
Subject: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Sat, 4 Feb 2023 12:59:10 +0100

Jostein, thank you for contributing a new compilation-mode pattern!

We generally want these regexps to be as tight as possible while still doing their job. This is partly because the large and ever-growing number of rules tend to interfere with one another, and over-broad patterns have shown to be a source of performance problems in the past.

I hope you don't mind helping us finishing the job:

First of all, both regexps match arbitrary amounts of horizontal whitespace at the beginning of a line, but neither message example you supplied contains any such leading whitespace. This means that either the set of test cases needs to be extended, or we could safely remove this leading whitespace matcher.

If leading whitespace indeed can occur, then when and how, exactly? Spaces or tabs, and how many? Please give us examples from actual compiler output.

Similarly the patterns match arbitrary whitespace before the word "error". This seems equally questionable -- would a single space do? If not, please provide actual output demonstrating it that could be added to the test suite, and tell us how it varies (tabs vs spaces, amount of whitespace, etc).

The following is a minor point that we'll fix but I thought you may want to know:

The use of [[:blank:]] and [[:alnum:]] is very likely more expensive than required since they accept Unicode whitespace and letters which obviously never will occur where matched so if it's all the same to you we'll reduce them to ASCII patterns.

Similarly, the inclusion of \r in patterns seems to be a misunderstanding: the tail part, "[^\r\n]+$", does not make sense -- normally, carriage returns aren't seen in buffers because line terminator translation convert everything to a single \n, and if a stray CR did occur then that pattern would never match anyway (why?).

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

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 Theodor Thornhill <theo <at> thornhill.no>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Sun, 5 Feb 2023 21:36:34 +0100

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

Hey there and thanks for the valuable feedback!

I'll try to do my best to provide the info I can, so that we can create 
the tightest regexps for this, which still are functional on the level 
users would expect.

On 2/4/23 12:59, Mattias Engdegård wrote:
> First of all, both regexps match arbitrary amounts of horizontal whitespace at the beginning of a line, but neither message example you supplied contains any such leading whitespace. This means that either the set of test cases needs to be extended, or we could safely remove this leading whitespace matcher.

I've gone looking, but I really can't find confirmation that this 
whitespace is required, at least not when building directly through the 
tsc TypeScript compiler.
I can see in the old test-suite for the MELPA package these two variants 
were the only test-cases present as well.

As such I think it's defiintely safe to remove this leading whitespace.

> Similarly the patterns match arbitrary whitespace before the word "error". This seems equally questionable -- would a single space do? If not, please provide actual output demonstrating it that could be added to the test suite, and tell us how it varies (tabs vs spaces, amount of whitespace, etc).
I can't see any real use-case for this either. Let's snip it.
> The following is a minor point that we'll fix but I thought you may want to know:
>
> The use of [[:blank:]] and [[:alnum:]] is very likely more expensive than required since they accept Unicode whitespace and letters which obviously never will occur where matched so if it's all the same to you we'll reduce them to ASCII patterns.
I've given this a try, and it seems to work fine. I'm OK with such a change.
> Similarly, the inclusion of \r in patterns seems to be a misunderstanding: the tail part, "[^\r\n]+$", does not make sense -- normally, carriage returns aren't seen in buffers because line terminator translation convert everything to a single \n, and if a stray CR did occur then that pattern would never match anyway (why?).

Fair enough. I've changed the code to only looks for \n instead.

Attached is a patch which codifies all these changes, and from what I 
can tell, still does the job. You make take it as is, or you may further 
work on it, if you think that is still needed.

--
Jostein

[0001-Optimize-compilation-mode-expressions-for-TypeScript.patch (text/x-patch, attachment)]

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

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 Theodor Thornhill <theo <at> thornhill.no>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Mon, 6 Feb 2023 12:19:32 +0100

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

5 feb. 2023 kl. 21.36 skrev Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>:

> Attached is a patch which codifies all these changes, and from what I can tell, still does the job. You make take it as is, or you may further work on it, if you think that is still needed.

Thank you! I translated the regexps to rx (which I personally find more maintainable and has plenty of precedence in this context) and tightened them up further. They are now anchored at beginning-of-line again, and file names cannot start with whitespace (for disambiguation and speed).

I also removed the part that matches the actual message text since it isn't needed, and it would highlight (with an underline in the standard theme) the whole text which is a bit ungainly to read. If the message is multi-line, which earlier examples in this discussions indicated might be the case, then only the first would be highlighted this way which wasn't ideal either.

Patch attached, please tell me what you think.

Does tsc distinguish between warnings, errors, and 'informational' messages (such as locations of interest that are not errors in their own right)? The examples you supplied all had the word "error" in a prominent place.

It would be possible to join the two tsc rules into a single one which would be slightly faster, but having them separate could also be an advantage since it would allow for them to be disabled individually in case of clashes.

[0001-Simplify-typescript-compilation-mode-patterns-bug-61.patch (application/octet-stream, attachment)]

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

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 Theodor Thornhill <theo <at> thornhill.no>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Mon, 6 Feb 2023 18:05:52 +0100

On 2/6/23 12:19, Mattias Engdegård wrote:
> Thank you! I translated the regexps to rx (which I personally find 
> more maintainable and has plenty of precedence in this context) and 
> tightened them up further. They are now anchored at beginning-of-line 
> again, and file names cannot start with whitespace (for disambiguation 
> and speed). 
Thanks. These are all good changes.
> I also removed the part that matches the actual message text since it isn't needed, and it would highlight (with an underline in the standard theme) the whole text which is a bit ungainly to read. If the message is multi-line, which earlier examples in this discussions indicated might be the case, then only the first would be highlighted this way which wasn't ideal either.
Seconded. No issues with that either.
> Does tsc distinguish between warnings, errors, and 'informational' messages (such as locations of interest that are not errors in their own right)? The examples you supplied all had the word "error" in a prominent place.

I don't think so. There are code-analysis warnings which seems to be 
provided to your editor of choice through LSP. These can be made into a 
compilation-error with the appropriate config-flag, but I can't out of 
the blue find any "middle ground" where they are emitted compile-time as 
warnings.

Someone please correct me if I'm wrong.

> It would be possible to join the two tsc rules into a single one which would be slightly faster, but having them separate could also be an advantage since it would allow for them to be disabled individually in case of clashes.

To me that sounds like an optimization going one step too far. It will 
result in a more complex expression, which is harder to work 
with/maintain, and might also be more computationally expensive due to 
back-tracking complexity.

I personally think that having 2 self-documented expressions side by 
side works better, but I'm no authority on compilation-mode and 
performance :)

> Patch attached, please tell me what you think.
I've tested that patch myself, and from what I can tell, it still works 
just fine :)

--
Jostein

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

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: jostein <at> kjonigsen.net
Cc: casouri <at> gmail.com, 61104 <at> debbugs.gnu.org,
 Theodor Thornhill <theo <at> thornhill.no>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#61104: 29.0.60; typescript-ts-mode does not provide
 compilation-mode support
Date: Mon, 6 Feb 2023 18:34:40 +0100

6 feb. 2023 kl. 18.05 skrev Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>:

> I've tested that patch myself, and from what I can tell, it still works just fine

Excellent, now pushed to emacs-29. We're done!

Previous Next

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