Re: master 0d5036061b5: Skip Eglot test if using very old clangd

Re: master 0d5036061b5: Skip Eglot test if using very old clangd

[Robert Pluim]

>>>>> On Tue, 21 Mar 2023 14:43:32 -0400 (EDT), João Távora <joaotavora@gmail.com> said:

    João> branch: master
    João> commit 0d5036061b544f5a306ccd275502f608ec9d3f25
    João> Author: João Távora <joaotavora@gmail.com>
    João> Commit: João Távora <joaotavora@gmail.com>

    João>     Skip Eglot test if using very old clangd
    
    João>     * test/lisp/progmodes/eglot-tests.el (eglot--clangd-version): New
    João>     helper.
    João>     (eglot-test-diagnostic-tags-unnecessary-code): Use it.
    João> ---
    João>  test/lisp/progmodes/eglot-tests.el | 7 +++++++
    João>  1 file changed, 7 insertions(+)

    João> diff --git a/test/lisp/progmodes/eglot-tests.el b/test/lisp/progmodes/eglot-tests.el
    João> index 7ac26732737..c4b23bfd64e 100644
    João> --- a/test/lisp/progmodes/eglot-tests.el
    João> +++ b/test/lisp/progmodes/eglot-tests.el
    João> @@ -314,6 +314,12 @@ then restored."
    João>    (setq last-command-event char)
    João>    (call-interactively (key-binding (vector char))))
 
    João> +(defun eglot--clangd-version ()
    João> +  "Report on the clangd version used in various tests."
    João> +  (replace-regexp-in-string
    João> +   ".*version[[:space:]]+\\(.*\\)" "\\1"
    João> +   (car (split-string (shell-command-to-string "clangd --version") "\n"))))

Not all clangd have the version string and only the version string at
the end of 'clangd --version'. Hereʼs one I installed from git earlier:

clangd version 16.0.0 (https://github.com/llvm/llvm-project.git 08d094a0e457360ad8b94b017d2dc277e697ca76)
Features: linux
Platform: x86_64-unknown-linux-gnu

So how about the following (or we could just say 'everything
non-space', but Iʼm assuming clangd versions are always made up of
numbers and '.')

diff --git c/test/lisp/progmodes/eglot-tests.el i/test/lisp/progmodes/eglot-tests.el
index c4b23bfd64e..557bd899ab7 100644
--- c/test/lisp/progmodes/eglot-tests.el
+++ i/test/lisp/progmodes/eglot-tests.el
@@ -317,7 +317,7 @@ eglot--simulate-key-event
 (defun eglot--clangd-version ()
   "Report on the clangd version used in various tests."
   (replace-regexp-in-string
-   ".*version[[:space:]]+\\(.*\\)" "\\1"
+   ".*version[[:space:]]+\\([0-9.]*\\).*" "\\1"
    (car (split-string (shell-command-to-string "clangd --version") "\n"))))
 
 \f


Robert
-- 

Re: master 0d5036061b5: Skip Eglot test if using very old clangd

[João Távora]

On Wed, Mar 22, 2023 at 10:38 AM Robert Pluim <rpluim@gmail.com> wrote:

> Not all clangd have the version string and only the version string at
> the end of 'clangd --version'. Hereʼs one I installed from git earlier:
>
> clangd version 16.0.0 (https://github.com/llvm/llvm-project.git 08d094a0e457360ad8b94b017d2dc277e697ca76)
> Features: linux
> Platform: x86_64-unknown-linux-gnu
>
> So how about the following (or we could just say 'everything
> non-space', but Iʼm assuming clangd versions are always made up of
> numbers and '.')
>
> diff --git c/test/lisp/progmodes/eglot-tests.el i/test/lisp/progmodes/eglot-tests.el
> index c4b23bfd64e..557bd899ab7 100644
> --- c/test/lisp/progmodes/eglot-tests.el
> +++ i/test/lisp/progmodes/eglot-tests.el
> @@ -317,7 +317,7 @@ eglot--simulate-key-event
>  (defun eglot--clangd-version ()
>    "Report on the clangd version used in various tests."
>    (replace-regexp-in-string
> -   ".*version[[:space:]]+\\(.*\\)" "\\1"
> +   ".*version[[:space:]]+\\([0-9.]*\\).*" "\\1"
>     (car (split-string (shell-command-to-string "clangd --version") "\n"))))


Sure, thanks.  Bonus points for getting rid of the split-string
and using a regexp that looks at the whole string and
catches the version.  I tried and failed.

João

Re: master 0d5036061b5: Skip Eglot test if using very old clangd

[Robert Pluim]

>>>>> On Wed, 22 Mar 2023 10:49:41 +0000, João Távora <joaotavora@gmail.com> said:

    João> On Wed, Mar 22, 2023 at 10:38 AM Robert Pluim <rpluim@gmail.com> wrote:
    >> Not all clangd have the version string and only the version string at
    >> the end of 'clangd --version'. Hereʼs one I installed from git earlier:
    >> 
    >> clangd version 16.0.0 (https://github.com/llvm/llvm-project.git 08d094a0e457360ad8b94b017d2dc277e697ca76)
    >> Features: linux
    >> Platform: x86_64-unknown-linux-gnu
    >> 
    >> So how about the following (or we could just say 'everything
    >> non-space', but Iʼm assuming clangd versions are always made up of
    >> numbers and '.')
    >> 
    >> diff --git c/test/lisp/progmodes/eglot-tests.el i/test/lisp/progmodes/eglot-tests.el
    >> index c4b23bfd64e..557bd899ab7 100644
    >> --- c/test/lisp/progmodes/eglot-tests.el
    >> +++ i/test/lisp/progmodes/eglot-tests.el
    >> @@ -317,7 +317,7 @@ eglot--simulate-key-event
    >> (defun eglot--clangd-version ()
    >> "Report on the clangd version used in various tests."
    >> (replace-regexp-in-string
    >> -   ".*version[[:space:]]+\\(.*\\)" "\\1"
    >> +   ".*version[[:space:]]+\\([0-9.]*\\).*" "\\1"
    >> (car (split-string (shell-command-to-string "clangd --version") "\n"))))


    João> Sure, thanks.  Bonus points for getting rid of the split-string
    João> and using a regexp that looks at the whole string and
    João> catches the version.  I tried and failed.

This doesnʼt care where 'version ' appears:

(defun eglot--clangd-version ()
  "Report on the clangd version used in various tests."
  (let ((version (shell-command-to-string "clangd --version")))
    (when (string-match "version[[:space:]]+\\([0-9.]*\\)"
                        version)
      (match-string 1 version))))

Robert
-- 

Re: master 0d5036061b5: Skip Eglot test if using very old clangd

[João Távora]

On Wed, Mar 22, 2023 at 11:12 AM Robert Pluim <rpluim@gmail.com> wrote:
>
> >>>>> On Wed, 22 Mar 2023 10:49:41 +0000, João Távora <joaotavora@gmail.com> said:
>
>     João> On Wed, Mar 22, 2023 at 10:38 AM Robert Pluim <rpluim@gmail.com> wrote:
>     >> Not all clangd have the version string and only the version string at
>     >> the end of 'clangd --version'. Hereʼs one I installed from git earlier:
>     >>
>     >> clangd version 16.0.0 (https://github.com/llvm/llvm-project.git 08d094a0e457360ad8b94b017d2dc277e697ca76)
>     >> Features: linux
>     >> Platform: x86_64-unknown-linux-gnu
>     >>
>     >> So how about the following (or we could just say 'everything
>     >> non-space', but Iʼm assuming clangd versions are always made up of
>     >> numbers and '.')
>     >>
>     >> diff --git c/test/lisp/progmodes/eglot-tests.el i/test/lisp/progmodes/eglot-tests.el
>     >> index c4b23bfd64e..557bd899ab7 100644
>     >> --- c/test/lisp/progmodes/eglot-tests.el
>     >> +++ i/test/lisp/progmodes/eglot-tests.el
>     >> @@ -317,7 +317,7 @@ eglot--simulate-key-event
>     >> (defun eglot--clangd-version ()
>     >> "Report on the clangd version used in various tests."
>     >> (replace-regexp-in-string
>     >> -   ".*version[[:space:]]+\\(.*\\)" "\\1"
>     >> +   ".*version[[:space:]]+\\([0-9.]*\\).*" "\\1"
>     >> (car (split-string (shell-command-to-string "clangd --version") "\n"))))
>
>
>     João> Sure, thanks.  Bonus points for getting rid of the split-string
>     João> and using a regexp that looks at the whole string and
>     João> catches the version.  I tried and failed.
>
> This doesnʼt care where 'version ' appears:
>
> (defun eglot--clangd-version ()
>   "Report on the clangd version used in various tests."
>   (let ((version (shell-command-to-string "clangd --version")))
>     (when (string-match "version[[:space:]]+\\([0-9.]*\\)"
>                         version)
>       (match-string 1 version))))

Looks good, can you push?  I wonder why this is so difficult
to do with replace-regexp-in-string.

João

Re: master 0d5036061b5: Skip Eglot test if using very old clangd

[Robert Pluim]

>>>>> On Wed, 22 Mar 2023 11:14:05 +0000, João Távora <joaotavora@gmail.com> said:
    >> This doesnʼt care where 'version ' appears:
    >> 
    >> (defun eglot--clangd-version ()
    >> "Report on the clangd version used in various tests."
    >> (let ((version (shell-command-to-string "clangd --version")))
    >> (when (string-match "version[[:space:]]+\\([0-9.]*\\)"
    >> version)
    >> (match-string 1 version))))

    João> Looks good, can you push?

Done

    João> I wonder why this is so difficult
    João> to do with replace-regexp-in-string.

Itʼs because we donʼt have a 'really-match-everything' version of '.',
so you end up with

  (replace-regexp-in-string
   ".*version[[:space:]]+\\([0-9.]*\\)\\(.\\|[\n\r]\\)*" "\\1"
   (shell-command-to-string "clangd --version"))

which I personally donʼt like the look of.

Robert
-- 

Re: master 0d5036061b5: Skip Eglot test if using very old clangd

[João Távora]

Robert Pluim <rpluim@gmail.com> writes:

>     João> I wonder why this is so difficult
>     João> to do with replace-regexp-in-string.
>
> Itʼs because we donʼt have a 'really-match-everything' version of '.',
> so you end up with
>
>   (replace-regexp-in-string
>    ".*version[[:space:]]+\\([0-9.]*\\)\\(.\\|[\n\r]\\)*" "\\1"
>    (shell-command-to-string "clangd --version"))
>
> which I personally donʼt like the look of.

I concur, your version looks much nicer.  Thanks again.

João