bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support

["Jostein Kjønigsen" <jostein@secure.kjonigsen.net>]

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

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

[-- Attachment #2: 0001-Optimize-compilation-mode-expressions-for-TypeScript.patch --]
[-- Type: text/x-patch, Size: 1488 bytes --]

From 1c6a71cd1db5b589ff9fc5f4fe76e9357b7bedbd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jostein=20Kj=C3=B8nigsen?= <jostein@kjonigsen.net>
Date: Sun, 5 Feb 2023 21:34:08 +0100
Subject: [PATCH] Optimize compilation-mode expressions for TypeScript

- lisp/progmodes/compile.el: remove unneeded and expensive checks.
---
 lisp/progmodes/compile.el | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 1e57d0b7bb..7700e5f7b1 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -654,18 +654,16 @@ compilation-error-regexp-alist-alist
     ;; greeter.ts(30,12): error TS2339: Property 'foo' does not exist.
     (typescript-tsc-plain
      ,(concat
-      "^[[:blank:]]*"
-      "\\([^(\r\n)]+\\)(\\([0-9]+\\),\\([0-9]+\\)):[[:blank:]]+"
-      "error [[:alnum:]]+: [^\r\n]+$")
+      "\\([^(\r\n)]+\\)(\\([0-9]+\\),\\([0-9]+\\)): "
+      "error [A-Z0-9]+: [^\n]+$")
      1 2 3 2)
 
     ;; Typescript compilation after tsc version 2.7, "pretty" format:
     ;; src/resources/document.ts:140:22 - error TS2362: something.
     (typescript-tsc-pretty
      ,(concat
-       "^[[:blank:]]*"
-       "\\([^(\r\n)]+\\):\\([0-9]+\\):\\([0-9]+\\) - [[:blank:]]*"
-       "error [[:alnum:]]+: [^\r\n]+$")
+       "\\([^(\r\n)]+\\):\\([0-9]+\\):\\([0-9]+\\) - "
+       "error [A-Z0-9]+: [^\n]+$")
      1 2 3 2)
     ))
   "Alist of values for `compilation-error-regexp-alist'.")
-- 
2.39.1