* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support @ 2023-01-27 20:14 Jostein Kjønigsen 2023-01-27 20:30 ` Eli Zaretskii 2023-02-04 11:59 ` Mattias Engdegård 0 siblings, 2 replies; 15+ messages in thread From: Jostein Kjønigsen @ 2023-01-27 20:14 UTC (permalink / raw) To: 61104; +Cc: Yuan Fu, Theodor Thornhill [-- Attachment #1: Type: text/plain, Size: 9441 bytes --] 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@kjonigsen.net 🍵 jostein@gmail.com https://jostein.kjønigsen.no <https://jostein.kjønigsen.no> [-- Attachment #2: Type: text/html, Size: 13086 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-01-27 20:14 bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support Jostein Kjønigsen @ 2023-01-27 20:30 ` Eli Zaretskii 2023-01-27 20:52 ` Jostein Kjønigsen 2023-02-04 11:59 ` Mattias Engdegård 1 sibling, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-27 20:30 UTC (permalink / raw) To: jostein; +Cc: casouri, 61104, theo > Cc: Yuan Fu <casouri@gmail.com>, Theodor Thornhill <theo@thornhill.no> > Date: Fri, 27 Jan 2023 21:14:30 +0100 > From: Jostein Kjønigsen <jostein@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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-01-27 20:30 ` Eli Zaretskii @ 2023-01-27 20:52 ` Jostein Kjønigsen 2023-01-28 7:23 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Jostein Kjønigsen @ 2023-01-27 20:52 UTC (permalink / raw) To: Eli Zaretskii, jostein; +Cc: casouri, 61104, theo 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-01-27 20:52 ` Jostein Kjønigsen @ 2023-01-28 7:23 ` Eli Zaretskii 2023-01-28 14:28 ` Jostein Kjønigsen 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-28 7:23 UTC (permalink / raw) To: jostein; +Cc: casouri, 61104, theo > Date: Fri, 27 Jan 2023 21:52:24 +0100 > Cc: 61104@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no > From: Jostein Kjønigsen <jostein@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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-01-28 7:23 ` Eli Zaretskii @ 2023-01-28 14:28 ` Jostein Kjønigsen 2023-02-02 21:01 ` Jostein Kjønigsen 0 siblings, 1 reply; 15+ messages in thread From: Jostein Kjønigsen @ 2023-01-28 14:28 UTC (permalink / raw) To: Eli Zaretskii, jostein; +Cc: casouri, 61104, theo [-- Attachment #1: Type: text/plain, Size: 1016 bytes --] 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 [-- Attachment #2: 0001-Add-support-for-Typescript-compilation-to-compilatio.patch --] [-- Type: text/x-patch, Size: 1476 bytes --] From b60c0686fc925290ff201ed79399e48ebc47d6d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jostein=20Kj=C3=B8nigsen?= <jostein@kjonigsen.net> Date: Sat, 28 Jan 2023 15:23:11 +0100 Subject: [PATCH] Add support for Typescript compilation to compilation-mode --- lisp/progmodes/compile.el | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el index 5758eadf996..1e57d0b7bb2 100644 --- a/lisp/progmodes/compile.el +++ b/lisp/progmodes/compile.el @@ -649,6 +649,24 @@ compilation-error-regexp-alist-alist ;; we do not know what lines will follow. (guile-file "^In \\(.+\\..+\\):\n" 1 nil nil 0) (guile-line "^ *\\([0-9]+\\): *\\([0-9]+\\)" nil 1 2) + + ;; Typescript compilation prior to tsc version 2.7, "plain" format: + ;; 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]+$") + 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]+$") + 1 2 3 2) )) "Alist of values for `compilation-error-regexp-alist'.") -- 2.39.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-01-28 14:28 ` Jostein Kjønigsen @ 2023-02-02 21:01 ` Jostein Kjønigsen 2023-02-03 5:30 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 15+ messages in thread From: Jostein Kjønigsen @ 2023-02-02 21:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: casouri, 61104, theo [-- Attachment #1: Type: text/plain, Size: 1293 bytes --] Any news on this one? Will this be merged? :) Vennlig hilsen *Jostein Kjønigsen* jostein@kjonigsen.net 🍵 jostein@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 [-- Attachment #2: Type: text/html, Size: 2449 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-02 21:01 ` Jostein Kjønigsen @ 2023-02-03 5:30 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-02-03 7:06 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-03 5:30 UTC (permalink / raw) To: jostein, Jostein Kjønigsen, Eli Zaretskii; +Cc: casouri, 61104 On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein@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@kjonigsen.net 🍵 jostein@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-03 5:30 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-03 7:06 ` Eli Zaretskii 2023-02-03 8:00 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-02-03 7:06 UTC (permalink / raw) To: Theodor Thornhill; +Cc: jostein, casouri, 61104, jostein > Date: Fri, 03 Feb 2023 06:30:29 +0100 > From: Theodor Thornhill <theo@thornhill.no> > CC: 61104@debbugs.gnu.org, casouri@gmail.com > > On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein@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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-03 7:06 ` Eli Zaretskii @ 2023-02-03 8:00 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-02-04 8:24 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 15+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-03 8:00 UTC (permalink / raw) To: Eli Zaretskii; +Cc: jostein, casouri, 61104, jostein Eli Zaretskii <eliz@gnu.org> writes: >> Date: Fri, 03 Feb 2023 06:30:29 +0100 >> From: Theodor Thornhill <theo@thornhill.no> >> CC: 61104@debbugs.gnu.org, casouri@gmail.com >> >> On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein@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))))) ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-03 8:00 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-04 8:24 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 15+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-04 8:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: jostein, casouri, 61104, jostein Theodor Thornhill <theo@thornhill.no> writes: > Eli Zaretskii <eliz@gnu.org> writes: > >>> Date: Fri, 03 Feb 2023 06:30:29 +0100 >>> From: Theodor Thornhill <theo@thornhill.no> >>> CC: 61104@debbugs.gnu.org, casouri@gmail.com >>> >>> On 2 February 2023 22:01:11 CET, "Jostein Kjønigsen" <jostein@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-01-27 20:14 bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support Jostein Kjønigsen 2023-01-27 20:30 ` Eli Zaretskii @ 2023-02-04 11:59 ` Mattias Engdegård 2023-02-05 20:36 ` Jostein Kjønigsen 1 sibling, 1 reply; 15+ messages in thread From: Mattias Engdegård @ 2023-02-04 11:59 UTC (permalink / raw) To: jostein; +Cc: casouri, 61104, Theodor Thornhill, Eli Zaretskii 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?). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-04 11:59 ` Mattias Engdegård @ 2023-02-05 20:36 ` Jostein Kjønigsen 2023-02-06 11:19 ` Mattias Engdegård 0 siblings, 1 reply; 15+ messages in thread From: Jostein Kjønigsen @ 2023-02-05 20:36 UTC (permalink / raw) To: Mattias Engdegård, jostein Cc: casouri, 61104, Theodor Thornhill, Eli Zaretskii [-- 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-05 20:36 ` Jostein Kjønigsen @ 2023-02-06 11:19 ` Mattias Engdegård 2023-02-06 17:05 ` Jostein Kjønigsen 0 siblings, 1 reply; 15+ messages in thread From: Mattias Engdegård @ 2023-02-06 11:19 UTC (permalink / raw) To: jostein; +Cc: casouri, 61104, Theodor Thornhill, Eli Zaretskii [-- Attachment #1: Type: text/plain, Size: 1451 bytes --] 5 feb. 2023 kl. 21.36 skrev Jostein Kjønigsen <jostein@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. [-- Attachment #2: 0001-Simplify-typescript-compilation-mode-patterns-bug-61.patch --] [-- Type: application/octet-stream, Size: 3086 bytes --] From d6d336cfe588dd5d8a5bf647c5beb9e45b47ef3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org> Date: Mon, 6 Feb 2023 11:45:33 +0100 Subject: [PATCH] Simplify typescript compilation-mode patterns (bug#61104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lisp/progmodes/compile.el (compilation-error-regexp-alist-alist): Tighten regexps and simplify. Translate to rx. * etc/compilation.txt: Add examples. In collaboration with Jostein Kjønigsen. --- etc/compilation.txt | 14 ++++++++++++++ lisp/progmodes/compile.el | 28 ++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/etc/compilation.txt b/etc/compilation.txt index 672cbebafff..5f6ecb09cc2 100644 --- a/etc/compilation.txt +++ b/etc/compilation.txt @@ -639,6 +639,20 @@ symbol: weblint index.html (13:1) Unknown element <fdjsk> +* Typescript prior to tsc version 2.7, "plain" format + +symbol: typescript-tsc-plain + +greeter.ts(30,12): error TS2339: Property 'foo' does not exist. + + +* Typescript after tsc version 2.7, "pretty" format + +symbol: typescript-tsc-pretty + +src/resources/document.ts:140:22 - error TS2362: something. + + * Directory tracking Directories are matched via 'compilation-directory-matcher'. Files which are diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el index 1e57d0b7bb2..ccf64fb670b 100644 --- a/lisp/progmodes/compile.el +++ b/lisp/progmodes/compile.el @@ -653,19 +653,31 @@ compilation-error-regexp-alist-alist ;; Typescript compilation prior to tsc version 2.7, "plain" format: ;; 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]+$") + ,(rx bol + (group (not (in " \t\n()")) ; 1: file + (* (not (in "\n()")))) + "(" + (group (+ (in "0-9"))) ; 2: line + "," + (group (+ (in "0-9"))) ; 3: column + "): error " + (+ (in "0-9A-Z")) ; error code + ": ") 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]+$") + ,(rx bol + (group (not (in " \t\n()")) ; 1: file + (* (not (in "\n()")))) + ":" + (group (+ (in "0-9"))) ; 2: line + ":" + (group (+ (in "0-9"))) ; 3: column + " - error " + (+ (in "0-9A-Z")) ; error code + ": ") 1 2 3 2) )) "Alist of values for `compilation-error-regexp-alist'.") -- 2.32.0 (Apple Git-132) ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-06 11:19 ` Mattias Engdegård @ 2023-02-06 17:05 ` Jostein Kjønigsen 2023-02-06 17:34 ` Mattias Engdegård 0 siblings, 1 reply; 15+ messages in thread From: Jostein Kjønigsen @ 2023-02-06 17:05 UTC (permalink / raw) To: Mattias Engdegård, jostein Cc: casouri, 61104, Theodor Thornhill, Eli Zaretskii 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support 2023-02-06 17:05 ` Jostein Kjønigsen @ 2023-02-06 17:34 ` Mattias Engdegård 0 siblings, 0 replies; 15+ messages in thread From: Mattias Engdegård @ 2023-02-06 17:34 UTC (permalink / raw) To: jostein; +Cc: casouri, 61104, Theodor Thornhill, Eli Zaretskii 6 feb. 2023 kl. 18.05 skrev Jostein Kjønigsen <jostein@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! ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-02-06 17:34 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-27 20:14 bug#61104: 29.0.60; typescript-ts-mode does not provide compilation-mode support Jostein Kjønigsen 2023-01-27 20:30 ` Eli Zaretskii 2023-01-27 20:52 ` Jostein Kjønigsen 2023-01-28 7:23 ` Eli Zaretskii 2023-01-28 14:28 ` Jostein Kjønigsen 2023-02-02 21:01 ` Jostein Kjønigsen 2023-02-03 5:30 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-02-03 7:06 ` Eli Zaretskii 2023-02-03 8:00 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-02-04 8:24 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-02-04 11:59 ` Mattias Engdegård 2023-02-05 20:36 ` Jostein Kjønigsen 2023-02-06 11:19 ` Mattias Engdegård 2023-02-06 17:05 ` Jostein Kjønigsen 2023-02-06 17:34 ` Mattias Engdegård
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.