From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?Q?Jostein_Kj=C3=B8nigsen?= Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] feat: add markdown-ts-mode Date: Sat, 20 Apr 2024 19:45:05 +0200 Message-ID: References: <877cgs7m4e.fsf@gmail.com> <86frvga5zc.fsf@gnu.org> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.300.61.1.2\)) Content-Type: multipart/alternative; boundary="Apple-Mail=_87B3EC16-7809-4D3C-94C8-40FE3B6DDB14" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="6464"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Yuan Fu , "Ergus via Emacs development discussions." , Eli Zaretskii To: Rahul Martim Juliato Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Apr 20 19:46:06 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ryEmq-0001Ry-GE for ged-emacs-devel@m.gmane-mx.org; Sat, 20 Apr 2024 19:46:05 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ryEmE-0000y1-Oc; Sat, 20 Apr 2024 13:45:27 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ryEmC-0000wQ-8n for emacs-devel@gnu.org; Sat, 20 Apr 2024 13:45:24 -0400 Original-Received: from wfhigh3-smtp.messagingengine.com ([64.147.123.154]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ryEm8-00063t-6l; Sat, 20 Apr 2024 13:45:23 -0400 Original-Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailfhigh.west.internal (Postfix) with ESMTP id 2C2DB18000C5; Sat, 20 Apr 2024 13:45:15 -0400 (EDT) Original-Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Sat, 20 Apr 2024 13:45:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= secure.kjonigsen.net; h=cc:cc:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1713635114; x=1713721514; bh=zL1WgqZUzzCiM8sRO1c8y4G2qrWAUu7xfPhWsnKr9ng=; b= F8/rTNgzjLnjRDG60Z5ia0QaocSPtVUxL0zcEJ8D1xQA4iz8tSIZryqdrVNFEgXW QFTfi3Xstrjv+/zi0u52QhB/sELz3GfcxUvU7yDtnPrBR4XDdW9yldWuDIWsKHuK I9UuubUkEAQN/7AFiQ5F6fewGUPBfGs24q4wMiyP126meLv1B6F/4UvirenKovUC InLcS8PAXd79EHuIKq2cYujfR74KzFoxF49+jvVOYCkg7F0uWgtg71e8sxi+Pt7C 73ycRQZEwGmvRYozQcT5SHUiAWbtDjmrBdTrIK4giOSUXq83fC/WttU5bCan5VJo esMLmlkzm8lF2WrRpGOifQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1713635114; x=1713721514; bh=zL1WgqZUzzCiM8sRO1c8y4G2qrWA Uu7xfPhWsnKr9ng=; b=BVTRrq0nW4imyG5TziNGMUJ4PNf5SXnldje73r/9SWej nWMgf58JaLlBa52KXfXpBnMuykQQIP7WNVamdxCRLX56JNhYdVoGvBnz6DLLGqyA HU7L3Z/wcrDweX2Oct7eP6yxvOGKUqZvthePHz8N6MiCuw7KiqNipl6cGIHtjdEO Di0n/SPvMvSw6DZewKS6l2mqFzHEYyTGVPYaj9PEEBxzidsvEZ2rtU6R0JEW/v+E L/Iwc/qdxn4rlGK2imjS6pD4k/IfUOHsIFbGlA/6ieoBzAie9mESWJR6AfglyK0Z qOd9sYyWKdukANIqu5e5waz7l745ZfudCMca0hlizg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudekgedgudduiecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhkfgtggfuffgjvefvfhfosegrtdhmrehhtdejnecuhfhrohhmpeflohhs thgvihhnucfmjhppnhhighhsvghnuceojhhoshhtvghinhesshgvtghurhgvrdhkjhhonh highhsvghnrdhnvghtqeenucggtffrrghtthgvrhhnpeehgeejleejjefhudfgveffkeei feefkeevkeetjeetteevgfelhfffleetgefgueenucffohhmrghinhepghhithhhuhgsrd gtohhmnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhep jhhoshhtvghinhesshgvtghurhgvrdhkjhhonhhighhsvghnrdhnvght X-ME-Proxy: Feedback-ID: ib2f84088:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat, 20 Apr 2024 13:45:13 -0400 (EDT) In-Reply-To: <86frvga5zc.fsf@gnu.org> X-Mailer: Apple Mail (2.3774.300.61.1.2) Received-SPF: pass client-ip=64.147.123.154; envelope-from=jostein@secure.kjonigsen.net; helo=wfhigh3-smtp.messagingengine.com X-Spam_score_int: -26 X-Spam_score: -2.7 X-Spam_bar: -- X-Spam_report: (-2.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:317890 Archived-At: --Apple-Mail=_87B3EC16-7809-4D3C-94C8-40FE3B6DDB14 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Some comments so far about *how* the mode should correctly be added to = repo, so I won't delve into that. Instead, I like the idea behind having built in modes for stuff like = this. So I went ahead and tested the real-world functionality of the = major-mode on the files I have :) First I tried to install the grammar using the default for = treesit-install-grammar, and that failed. I then tweaked some settings, = and I think I did the right thing? Repo: https://github.com/tree-sitter-grammars/tree-sitter-markdown Subfolder for compilation: tree-sitter-markdown/src After installing this grammar using , I can compile/evaluate the = provided major-mode (but not before!). I think most other TS-based = major-modes in Emacs have a way to handle this more gracefully, and that = should probably be addressed here too? Now after compiling it and activating it... Im getting some confusing = behaviour on existing files: 1. No syntax-highlighting at all. Not on new, nor existing buffers. What = I am doing wrong here? :) 2. Newline chars in modeline to tell me my current section (# = Deployment^J). Is this major-mode somehow linebreak-type sensitive? IMO = that's not a great thing. 3. No imenu? Imenu really would make this nicer to use, but not really = worth looking into until other issues are resolved. The contrast to the version you say you've already published on MELPA is = quite significant, where from what I can tell...=20 In that mode, literally everything works. Are there any reason you can't = submit that version instead of this modified version? Also: Please don't consider these comments in a dissuading manner. These = are meant as constructive criticism, because I want Emacs to have these = things, but then they need to be good enough :) =E2=80=94 Kind Regards Jostein Kj=C3=B8nigsen > On 20 Apr 2024, at 08:44, Eli Zaretskii wrote: >=20 >> From: Rahul Martim Juliato >> Date: Sat, 20 Apr 2024 00:23:45 -0300 >>=20 >> I've been using Emacs without any extra packages as an educational >> experiment after years of package hording. >>=20 >>=20 >> One of the few things I've been missing is a way of displaying some = sort >> of syntax highlight for markdown documents. >>=20 >>=20 >> It feels a bit frustrating opening a README.md file with a single = face >> color, since Emacs from the box can handle so much. I am sure others >> might have shared this feeling before. >>=20 >>=20 >> This patch is a modified version of a package I've recently published = on >> MELPA, screenshots are available here: >> https://github.com/LionyxML/markdown-ts-mode >>=20 >>=20 >> It is a very basic mode that provides syntax highlight and imenu = support >> using treesitter grammar from >> https://github.com/tree-sitter-grammars/tree-sitter-markdown >>=20 >>=20 >> The idea here is to provide minimal support to Emacs and continue >> building up features in the future. >>=20 >>=20 >> It is the first time I contribute to Emacs devel, so please let me = know >> iif I did something wrong or anything is not at the expected = standards. >=20 > Thanks, please see a couple of comments below. I've CC'ed Yuan as > well, in case he has comments. >=20 >>> =46rom 364f61b03d601d2cb3aeb1687da2d1b2a232474c Mon Sep 17 00:00:00 = 2001 >> From: Rahul Martim Juliato >> Date: Fri, 19 Apr 2024 23:21:20 -0300 >>=20 >> --- >> etc/NEWS | 5 ++ >> lisp/progmodes/markdown-ts-mode.el | 106 = +++++++++++++++++++++++++++++ >> 2 files changed, 111 insertions(+) >> create mode 100644 lisp/progmodes/markdown-ts-mode.el >=20 > Please accompany the patches with a ChangeLog-style commit log > message; see CONTRIBUTE for the details. In this case, you'd need a > very minimal one, something like: >=20 > * lisp/progmodes/markdown-ts-mode.el: New file. > * etc/NEWS: Announce the new mode. >=20 >> +--- >> +*** New major mode 'markdown-ts-mode'. >> +A major mode based on the tree-sitter library for editing Markdown = files. >=20 > How about mentioning this in the user manual as well? It doesn't have > to be anything more than just the name of the mode. >=20 >> diff --git a/lisp/progmodes/markdown-ts-mode.el = b/lisp/progmodes/markdown-ts-mode.el >=20 > Should this mode live in lisp/textmodes/ instead? Markdown is AFAIU a > mode for text with markup, it isn't a programming language. >=20 >> +(defun markdown-ts-setup () >> + "Setup treesit for `markdown-ts-mode'." >> + (setq-local treesit-font-lock-settings = markdown-ts--treesit-settings) >> + (treesit-major-mode-setup)) >> + >> +;;;###autoload >> +(define-derived-mode markdown-ts-mode fundamental-mode = "markdown[ts]" >> + "Major mode for editing Markdown using tree-sitter grammar." >> + (setq-local font-lock-defaults nil >> + treesit-font-lock-feature-list '((delimiter) >> + (paragraph))) >=20 > I wonder whether this mode should inherit from Text mode instead, and > consequently have some text-related commands, perhaps aided by the > tree-sitter grammar? WDYT? We could, of course, add commands later, > but the decision to have Text mode as the parent of this one should be > made now. >=20 --Apple-Mail=_87B3EC16-7809-4D3C-94C8-40FE3B6DDB14 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
Some = comments so far about *how* the mode should correctly be added to repo, = so I won't delve into that.

Instead, I like the idea = behind having built in modes for stuff like this. So I went ahead and = tested the real-world functionality of the major-mode on the files I = have :)

First I tried to install the grammar using = the default for treesit-install-grammar, and that failed. I then tweaked = some settings, and I think I did the right = thing?

Sub= folder for compilation: 

tree-sitter-markdown/src


After = installing this grammar using , I can compile/evaluate the provided = major-mode (but not before!). I think most other TS-based major-modes in = Emacs have a way to handle this more gracefully, and that should = probably be addressed here too?

Now after = compiling it and activating it... Im getting some confusing behaviour on = existing files:

1. No syntax-highlighting at = all. Not on new, nor existing buffers. What I am doing wrong here? = :)
2. Newline chars in modeline to tell me my current section = (# Deployment^J). Is this major-mode somehow linebreak-type sensitive? = IMO that's not a great thing.
3. No imenu? Imenu really would = make this nicer to use, but not really worth looking into until other = issues are resolved.

The contrast to the = version you say you've already published on MELPA is quite significant, = where from what I can tell... 

In that = mode, literally everything works. Are there any reason you can't submit = that version instead of this modified = version?

Also: Please don't consider these = comments in a dissuading manner. These are meant as constructive = criticism, because I want Emacs to have these things, but then they need = to be good enough :)


=E2=80=94
Kind Regards
Jostein = Kj=C3=B8nigsen

On 20 Apr 2024, at 08:44, Eli = Zaretskii <eliz@gnu.org> wrote:

From: Rahul Martim Juliato = <rahuljuliato@gmail.com>
Date: Sat, 20 Apr 2024 00:23:45 = -0300

I've been using Emacs without any extra packages as an = educational
experiment after years of package hording.


One = of the few things I've been missing is a way of displaying some = sort
of syntax highlight for markdown documents.


It feels = a bit frustrating opening a README.md file with a single face
color, = since Emacs from the box can handle so much.  I am sure = others
might have shared this feeling before.


This patch = is a modified version of a package I've recently published on
MELPA, = screenshots are available = here:
https://github.com/LionyxML/markdown-ts-mode


It is a = very basic mode that provides syntax highlight and imenu = support
using treesitter grammar = from
https://github.com/tree-sitter-grammars/tree-sitter-markdown

The idea here is to provide minimal support to Emacs and = continue
building up features in the future.


It is the = first time I contribute to Emacs devel, so please let me know
iif I = did something wrong or anything is not at the expected = standards.

Thanks, please see a couple of comments = below.  I've CC'ed Yuan as
well, in case he has = comments.

=46ro= m 364f61b03d601d2cb3aeb1687da2d1b2a232474c Mon Sep 17 00:00:00 = 2001
From: Rahul Martim Juliato = <rahul.juliato@gmail.com>
Date: Fri, 19 Apr 2024 23:21:20 = -0300

---
etc/NEWS =             &n= bsp;           &nbs= p; |   5 ++
lisp/progmodes/markdown-ts-mode.el | 106 = +++++++++++++++++++++++++++++
2 files changed, 111 insertions(+)
= create mode 100644 = lisp/progmodes/markdown-ts-mode.el

Please accompany = the patches with a ChangeLog-style commit log
message; see CONTRIBUTE = for the details.  In this case, you'd need a
very minimal one, = something like:

 * lisp/progmodes/markdown-ts-mode.el: New = file.
 * etc/NEWS: Announce the new mode.

+---
+*** New major mode 'markdown-ts-mode'.
+A = major mode based on the tree-sitter library for editing Markdown = files.

How about mentioning this in the user manual = as well?  It doesn't have
to be anything more than just the name = of the mode.

diff --git = a/lisp/progmodes/markdown-ts-mode.el = b/lisp/progmodes/markdown-ts-mode.el

Should this = mode live in lisp/textmodes/ instead?  Markdown is AFAIU a
mode = for text with markup, it isn't a programming = language.

+(defun markdown-ts-setup = ()
+  "Setup treesit for `markdown-ts-mode'."
+ =  (setq-local treesit-font-lock-settings = markdown-ts--treesit-settings)
+ =  (treesit-major-mode-setup))
+
+;;;###autoload
+(define-deri= ved-mode markdown-ts-mode fundamental-mode "markdown[ts]"
+ =  "Major mode for editing Markdown using tree-sitter grammar."
+ =  (setq-local font-lock-defaults nil
+ =      treesit-font-lock-feature-list = '((delimiter)
+ =       (paragraph)))

I = wonder whether this mode should inherit from Text mode instead, = and
consequently have some text-related commands, perhaps aided by = the
tree-sitter grammar?  WDYT?  We could, of course, add = commands later,
but the decision to have Text mode as the parent of = this one should be
made = now.


= --Apple-Mail=_87B3EC16-7809-4D3C-94C8-40FE3B6DDB14--