* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
@ 2024-06-04 2:51 Troy Brown
2024-06-04 5:41 ` Felician Nemeth
0 siblings, 1 reply; 13+ messages in thread
From: Troy Brown @ 2024-06-04 2:51 UTC (permalink / raw)
To: 71353
[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]
It appears that eglot--format-markup only supports MarkedString for
string literals and MarkupContent. This causes markup provided by the
server as a MarkedString code-block to be formatted using the major
mode. This happens because the MarkedString code-block uses the
"language" field to designate the type of markup.
eglot--format-markup is only looking for the "kind" field found in
MarkupContent. The following is an example of a hover response from
an LSP server using a MarkedString code-block to provide a description
in "plaintext".
[jsonrpc] e[13:34:28.888] --> textDocument/hover[25]
{"jsonrpc":"2.0","id":25,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///home/troy/repos/crates/gtkada_24.0.0_80c56171/src/gtkada.gpr"},"position":{"line":70,"character":12}}}
[jsonrpc] e[13:34:29.033] <-- textDocument/hover[25]
{"jsonrpc":"2.0","id":25,"result":{"contents":[{"language":"plaintext","value":"This
package specifies the compilation options used when building an
executable or a library for a project. Most of the options should be
set in one of Compiler, Binder or Linker packages, but there are some
general options that should be defined in this package."}]}}
The following patch will format MarkedString code-blocks for markdown
and plaintext while using the major mode to format any other language.
[-- Attachment #2: 0001-Eglot-Support-formatting-MarkedString-code-block.patch --]
[-- Type: text/x-patch, Size: 1124 bytes --]
From 4a9412478819c6cb4446d2169901cdb9152a203a Mon Sep 17 00:00:00 2001
From: Troy Brown <brownts@troybrown.dev>
Date: Mon, 3 Jun 2024 22:19:24 -0400
Subject: [PATCH] Eglot: Support formatting MarkedString code-block
* lisp/progmodes/eglot.el (eglot--format-markup): Add support
for MarkedString code-block.
Copyright-paperwork-exempt: yes
---
lisp/progmodes/eglot.el | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 5ccae5210fe..3a44acdb75b 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1862,7 +1862,8 @@ eglot--format-markup
(pcase-let ((`(,string ,mode)
(if (stringp markup) (list markup 'gfm-view-mode)
(list (plist-get markup :value)
- (pcase (plist-get markup :kind)
+ (pcase (or (plist-get markup :kind)
+ (plist-get markup :language))
("markdown" 'gfm-view-mode)
("plaintext" 'text-mode)
(_ major-mode))))))
--
2.37.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-04 2:51 bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks Troy Brown
@ 2024-06-04 5:41 ` Felician Nemeth
2024-06-04 10:01 ` Troy Brown
0 siblings, 1 reply; 13+ messages in thread
From: Felician Nemeth @ 2024-06-04 5:41 UTC (permalink / raw)
To: Troy Brown; +Cc: 71353
Troy Brown <brownts@troybrown.dev> writes:
> The following patch will format MarkedString code-blocks for markdown
> and plaintext while using the major mode to format any other language.
According to the LSP specification, if MarkedString is "{ language:
string; value: string }", then it should be interpreted as this markdown
formatted string:
```${language}
${value}
```
The patch does not implement this behavior. And as a sidenote,
MarkedString is deprecated.
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_hover
^ permalink raw reply [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-04 5:41 ` Felician Nemeth
@ 2024-06-04 10:01 ` Troy Brown
2024-06-04 11:21 ` Felician Nemeth
0 siblings, 1 reply; 13+ messages in thread
From: Troy Brown @ 2024-06-04 10:01 UTC (permalink / raw)
To: Felician Nemeth; +Cc: 71353
On Tue, Jun 4, 2024 at 1:42 AM Felician Nemeth
<felician.nemeth@gmail.com> wrote:
>
> According to the LSP specification, if MarkedString is "{ language:
> string; value: string }", then it should be interpreted as this markdown
> formatted string:
>
> ```${language}
> ${value}
> ```
>
> The patch does not implement this behavior. And as a sidenote,
> MarkedString is deprecated.
>
> https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_hover
>
Unless I'm mistaken, I believe it does implement that behavior if the
language is markdown, plaintext or the same as the major mode. This
seemed to be the intent of the existing implementation which doesn't
support any other options (and falls back to the major mode) and the
reason why I didn't expand on it with this patch. I agree that the
patch does not support anything outside that behavior. Additionally,
I don't think that because MarkedString is deprecated is a reason to
implement partial support for it (MarkedString for string literals)
when there are obviously existing LSP servers that utilize it.
^ permalink raw reply [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-04 10:01 ` Troy Brown
@ 2024-06-04 11:21 ` Felician Nemeth
2024-06-04 12:37 ` Troy Brown
0 siblings, 1 reply; 13+ messages in thread
From: Felician Nemeth @ 2024-06-04 11:21 UTC (permalink / raw)
To: Troy Brown; +Cc: 71353
Troy Brown <brownts@troybrown.dev> writes:
> On Tue, Jun 4, 2024 at 1:42 AM Felician Nemeth
> <felician.nemeth@gmail.com> wrote:
>>
>> According to the LSP specification, if MarkedString is "{ language:
>> string; value: string }", then it should be interpreted as this markdown
>> formatted string:
>>
>> ```${language}
>> ${value}
>> ```
> Unless I'm mistaken, I believe it does implement that behavior if the
> language is markdown, plaintext or the same as the major mode.
I don't think "markdown" is a valid markdown language.
For example, if a MarkedString is
{ "language": "c", "value": "printf(42);"}
then the LSP client should interpret it as this markdown-formatted text:
```c
printf(42);
```
Whereas I think you argue that this MarkedString:
{ "language": "markdown", "value": "```c\nprintf(42);\n```"}
should be interpreted as this markdown-formatted text:
```c
printf(42);
```
I beleive the second example is not in line with the LSP specification.
The patch, however, will format "printf(42);" according to the
major-mode of the current buffer, which is probably c-mode or something
similar. So this part will usually work.
In any case, why not just turn a MarkedString into a markdown-formatted
text and give it gfm-view-mode?
^ permalink raw reply [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-04 11:21 ` Felician Nemeth
@ 2024-06-04 12:37 ` Troy Brown
2024-06-04 19:36 ` Felician Nemeth
0 siblings, 1 reply; 13+ messages in thread
From: Troy Brown @ 2024-06-04 12:37 UTC (permalink / raw)
To: Felician Nemeth; +Cc: 71353
[-- Attachment #1: Type: text/plain, Size: 340 bytes --]
On Tue, Jun 4, 2024 at 7:22 AM Felician Nemeth
<felician.nemeth@gmail.com> wrote:
>
> In any case, why not just turn a MarkedString into a markdown-formatted
> text and give it gfm-view-mode?
Thanks for the detailed explanation. I've created a new patch which
creates a fenced code block and uses gfm-view-mode, as you suggest.
[-- Attachment #2: 0001-Eglot-Support-formatting-MarkedString-code-block.patch --]
[-- Type: text/x-patch, Size: 1676 bytes --]
From e9c6387eb123aac1fe4741a871be2b68128de41a Mon Sep 17 00:00:00 2001
From: Troy Brown <brownts@troybrown.dev>
Date: Tue, 4 Jun 2024 08:30:53 -0400
Subject: [PATCH] Eglot: Support formatting MarkedString code-block
* lisp/progmodes/eglot.el (eglot--format-markup): Add support
for MarkedString code-block.
Copyright-paperwork-exempt: yes
---
lisp/progmodes/eglot.el | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 5ccae5210fe..70224d0dcd6 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1861,11 +1861,15 @@ eglot--format-markup
"Format MARKUP according to LSP's spec."
(pcase-let ((`(,string ,mode)
(if (stringp markup) (list markup 'gfm-view-mode)
- (list (plist-get markup :value)
- (pcase (plist-get markup :kind)
- ("markdown" 'gfm-view-mode)
- ("plaintext" 'text-mode)
- (_ major-mode))))))
+ (if-let ((language (plist-get markup :language))
+ (value (plist-get markup :value)))
+ (list (concat "```" language "\n" value "\n```")
+ 'gfm-view-mode)
+ (list (plist-get markup :value)
+ (pcase (plist-get markup :kind)
+ ("markdown" 'gfm-view-mode)
+ ("plaintext" 'text-mode)
+ (_ major-mode)))))))
(with-temp-buffer
(setq-local markdown-fontify-code-blocks-natively t)
(insert string)
--
2.37.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-04 12:37 ` Troy Brown
@ 2024-06-04 19:36 ` Felician Nemeth
2024-06-15 8:12 ` Eli Zaretskii
0 siblings, 1 reply; 13+ messages in thread
From: Felician Nemeth @ 2024-06-04 19:36 UTC (permalink / raw)
To: Troy Brown; +Cc: 71353, João Távora
Troy Brown <brownts@troybrown.dev> writes:
> I've created a new patch which creates a fenced code block and uses
> gfm-view-mode,
I have not tried it, but it looks good. Maybe a comment would help to
understand that the new part handles the case when the MarkedString is a
code-block.
At any rate, I CC'd the maintainer of Eglot.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-04 19:36 ` Felician Nemeth
@ 2024-06-15 8:12 ` Eli Zaretskii
2024-06-15 9:37 ` João Távora
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2024-06-15 8:12 UTC (permalink / raw)
To: Felician Nemeth; +Cc: brownts, 71353, joaotavora
> Cc: 71353@debbugs.gnu.org,
> João Távora <joaotavora@gmail.com>
> From: Felician Nemeth <felician.nemeth@gmail.com>
> Date: Tue, 04 Jun 2024 21:36:02 +0200
>
> Troy Brown <brownts@troybrown.dev> writes:
>
> > I've created a new patch which creates a fenced code block and uses
> > gfm-view-mode,
>
> I have not tried it, but it looks good. Maybe a comment would help to
> understand that the new part handles the case when the MarkedString is a
> code-block.
>
> At any rate, I CC'd the maintainer of Eglot.
João, any comments or suggestions?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-15 8:12 ` Eli Zaretskii
@ 2024-06-15 9:37 ` João Távora
2024-06-15 12:36 ` Eli Zaretskii
0 siblings, 1 reply; 13+ messages in thread
From: João Távora @ 2024-06-15 9:37 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71353, Troy Brown, Felician Nemeth
[-- Attachment #1: Type: text/plain, Size: 722 bytes --]
Can you please resend the patch for inspection?
On Sat, Jun 15, 2024, 09:12 Eli Zaretskii <eliz@gnu.org> wrote:
> > Cc: 71353@debbugs.gnu.org,
> > João Távora <joaotavora@gmail.com>
> > From: Felician Nemeth <felician.nemeth@gmail.com>
> > Date: Tue, 04 Jun 2024 21:36:02 +0200
> >
> > Troy Brown <brownts@troybrown.dev> writes:
> >
> > > I've created a new patch which creates a fenced code block and uses
> > > gfm-view-mode,
> >
> > I have not tried it, but it looks good. Maybe a comment would help to
> > understand that the new part handles the case when the MarkedString is a
> > code-block.
> >
> > At any rate, I CC'd the maintainer of Eglot.
>
> João, any comments or suggestions?
>
[-- Attachment #2: Type: text/html, Size: 1394 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-15 9:37 ` João Távora
@ 2024-06-15 12:36 ` Eli Zaretskii
2024-06-18 13:39 ` João Távora
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2024-06-15 12:36 UTC (permalink / raw)
To: João Távora; +Cc: 71353, brownts, felician.nemeth
[-- Attachment #1: Type: text/plain, Size: 270 bytes --]
> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 15 Jun 2024 10:37:48 +0100
> Cc: Felician Nemeth <felician.nemeth@gmail.com>, Troy Brown <brownts@troybrown.dev>,
> 71353@debbugs.gnu.org
>
> Can you please resend the patch for inspection?
Attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Eglot-Support-formatting-MarkedString-code-block.patch --]
[-- Type: text/x-patch, Size: 1676 bytes --]
From e9c6387eb123aac1fe4741a871be2b68128de41a Mon Sep 17 00:00:00 2001
From: Troy Brown <brownts@troybrown.dev>
Date: Tue, 4 Jun 2024 08:30:53 -0400
Subject: [PATCH] Eglot: Support formatting MarkedString code-block
* lisp/progmodes/eglot.el (eglot--format-markup): Add support
for MarkedString code-block.
Copyright-paperwork-exempt: yes
---
lisp/progmodes/eglot.el | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 5ccae5210fe..70224d0dcd6 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1861,11 +1861,15 @@ eglot--format-markup
"Format MARKUP according to LSP's spec."
(pcase-let ((`(,string ,mode)
(if (stringp markup) (list markup 'gfm-view-mode)
- (list (plist-get markup :value)
- (pcase (plist-get markup :kind)
- ("markdown" 'gfm-view-mode)
- ("plaintext" 'text-mode)
- (_ major-mode))))))
+ (if-let ((language (plist-get markup :language))
+ (value (plist-get markup :value)))
+ (list (concat "```" language "\n" value "\n```")
+ 'gfm-view-mode)
+ (list (plist-get markup :value)
+ (pcase (plist-get markup :kind)
+ ("markdown" 'gfm-view-mode)
+ ("plaintext" 'text-mode)
+ (_ major-mode)))))))
(with-temp-buffer
(setq-local markdown-fontify-code-blocks-natively t)
(insert string)
--
2.37.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-15 12:36 ` Eli Zaretskii
@ 2024-06-18 13:39 ` João Távora
2024-07-01 3:28 ` Troy Brown
0 siblings, 1 reply; 13+ messages in thread
From: João Távora @ 2024-06-18 13:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71353, brownts, felician.nemeth
On Sat, Jun 15, 2024 at 1:36 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Sat, 15 Jun 2024 10:37:48 +0100
> > Cc: Felician Nemeth <felician.nemeth@gmail.com>, Troy Brown <brownts@troybrown.dev>,
> > 71353@debbugs.gnu.org
> >
> > Can you please resend the patch for inspection?
>
> Attached.
The gist of the patch is correct, but please consider replacing by
this alternative, I don't like if-let inside pcase-let, it's too
complex: the following patch should be "flatter".
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 6896baf30ce..eabe01a1676 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1863,14 +1863,22 @@ eglot--snippet-expansion-fn
(apply #'yas-expand-snippet args)))))
(defun eglot--format-markup (markup)
- "Format MARKUP according to LSP's spec."
- (pcase-let ((`(,string ,mode)
- (if (stringp markup) (list markup 'gfm-view-mode)
- (list (plist-get markup :value)
- (pcase (plist-get markup :kind)
- ("markdown" 'gfm-view-mode)
- ("plaintext" 'text-mode)
- (_ major-mode))))))
+ "Format MARKUP according to LSP's spec.
+MARKUP is either an LSP MarkedString or MarkupContent object."
+ (let (string mode language)
+ (cond ((stringp markup)
+ (setq string markup
+ mode 'gfm-view-mode))
+ ((setq language (plist-get markup :language))
+ (setq string (concat "```" language "\n"
+ (plist-get markup :value) "\n```")
+ mode 'gfm-view-mode))
+ (t
+ (setq string (plist-get markup :value)
+ mode (pcase (plist-get markup :kind)
+ ("markdown" 'gfm-view-mode)
+ ("plaintext" 'text-mode)
+ (_ major-mode)))))
(with-temp-buffer
(setq-local markdown-fontify-code-blocks-natively t)
(insert string)
^ permalink raw reply related [flat|nested] 13+ messages in thread* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-06-18 13:39 ` João Távora
@ 2024-07-01 3:28 ` Troy Brown
2024-07-06 8:41 ` Eli Zaretskii
0 siblings, 1 reply; 13+ messages in thread
From: Troy Brown @ 2024-07-01 3:28 UTC (permalink / raw)
To: João Távora; +Cc: 71353, Eli Zaretskii, felician.nemeth
On Tue, Jun 18, 2024 at 9:39 AM João Távora <joaotavora@gmail.com> wrote:
>
> The gist of the patch is correct, but please consider replacing by
> this alternative, I don't like if-let inside pcase-let, it's too
> complex: the following patch should be "flatter".
>
Thanks João, I've tested your patch and verified it addresses my
issue. Since I'm at my limit for patches without copyright
assignment, can you apply your correction for this?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks
2024-07-01 3:28 ` Troy Brown
@ 2024-07-06 8:41 ` Eli Zaretskii
2024-07-06 9:18 ` João Távora
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2024-07-06 8:41 UTC (permalink / raw)
To: joaotavora, Troy Brown; +Cc: 71353, felician.nemeth
> From: Troy Brown <brownts@troybrown.dev>
> Date: Sun, 30 Jun 2024 23:28:36 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, felician.nemeth@gmail.com, 71353@debbugs.gnu.org
>
> On Tue, Jun 18, 2024 at 9:39 AM João Távora <joaotavora@gmail.com> wrote:
> >
> > The gist of the patch is correct, but please consider replacing by
> > this alternative, I don't like if-let inside pcase-let, it's too
> > complex: the following patch should be "flatter".
> >
>
> Thanks João, I've tested your patch and verified it addresses my
> issue. Since I'm at my limit for patches without copyright
> assignment, can you apply your correction for this?
João, would you please install your patch?
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-07-06 9:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-04 2:51 bug#71353: [PATCH] eglot--format-markup doesn't support MarkedString code-blocks Troy Brown
2024-06-04 5:41 ` Felician Nemeth
2024-06-04 10:01 ` Troy Brown
2024-06-04 11:21 ` Felician Nemeth
2024-06-04 12:37 ` Troy Brown
2024-06-04 19:36 ` Felician Nemeth
2024-06-15 8:12 ` Eli Zaretskii
2024-06-15 9:37 ` João Távora
2024-06-15 12:36 ` Eli Zaretskii
2024-06-18 13:39 ` João Távora
2024-07-01 3:28 ` Troy Brown
2024-07-06 8:41 ` Eli Zaretskii
2024-07-06 9:18 ` João Távora
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.