Re: [PATCH] feat: add markdown-ts-mode

[Rahul Martim Juliato]

Thanks Eli and Jostein!

No problem criticising :)


Please find a new patch attached. Things I've made:

- changed the file to the textmodes folder

- made it inherit from `text-mode'

- changed the news with +++

- changed `emacs.texi' (is it here?) to add Markdown

- scratched my head until 2:30AM thinking why it would not work for
  Jostein test.


Well Jostein, I've been played by the tree-sitter github repository
maintainer, lol.

First, a little note. There's actually no major changes to the version
on github. I only changed documenting strings and made some small name
changes to more "alike" already existing modes.

It happens that on tree-sitter repository, unlike any other of their
repositories (that I've seen), "main" branch is NOT the current branch,
but the "we moved from here bye" one.

The current default branch is "split_parser" where
tree-sitter-markdown/src/ resides, hence the confusion.

So, I wrote the mode with the "main" one in mind.

I already checked the "new" (split_parser) one, and it seems it is
possible to convert the current work to use this one instead. I'll try
to do this in the middle of the week or sooner :)

In the mean time, I realized more people would like to test this patch
and tree-sitter setup is really trick, so I made this little "guide" on
how to apply it until the end result (this might work now, pointing to
the "main" branch with the current patch).

If you could please try the path (B) on the guide and tell me if this
works for you, It would be nice, Jostein.


Thanks!

Rahul



--- beggining of guide on how to apply this patch and test it

Git apply this patch.

Run the autogen script:

./autogen.sh

Make sure configure uses tree-sitter:

./configure --with-tree-sitter

Compile:

make bootstrap

Check the build:

./src/emacs -Q --version

Open emacs

./src/emacs -Q --init-dir=~/tmp_emacs_dir/


A) The Hard Way

Visit a .md file

C-x f TEST.md

Note the mode will not automatically load.

This behavior is the same as typescript-ts-mode or other
treesitter modes I've been using.

M-x markdown-ts-mode

It will fail since you have no tree sitter grammar installed.
and will suggest installing it with `treesit-install-language-grammar'.

This behaviour is also standard for the tree-sitter modes I
currently use (typescript, tsx, rust).

Issuing "M-x treesit-install-language-grammar RET".

It asks for language, complete with "markdown RET".

It says there's no recipie for it, if you want to build it
interactivelly. Anwser "yes".

It asks for the URL where the grammar is hosted, enter:
"https://github.com/tree-sitter-grammars/tree-sitter-markdown RET"

It asks for the tag or branch, setting the default, enter "main RET".

It asks for the subfolder, leave the default (src), enter "RET".

It asks for the C compiler to use (default: auto-detect), just "RET".

It asks for the C++ compiler to use (default: auto-detect), just "RET".

Install to (default "~/tmp_emacs_dir/treesitter"), just "RET".

It will clone the repository, compile the library and tell in the
minibuffer the library is installed to your folder.


B) The Easier Way.

Copy-paste and eval this use-package definition.

(use-package markdown-ts-mode
  :mode ("\\.md\\'" . markdown-ts-mode)
  :defer 't
  :config
  (add-to-list 'treesit-language-source-alist '(markdown 
"https://github.com/tree-sitter-grammars/tree-sitter-markdown"; "main" "src")))


Visit your markdown test file.

It will probably fail due to the missing grammar.

Issue "M-x treesit-install-language-grammar RET".

Now with TAB it should complete "markdown", if not, type it.

As we already have the source now defined, just hit "RET" to install
to the default treemacs folder.

It will clone the repository and say it installed on your folder.

Just reload the mode with "M-x markdown-ts-mode".

>From now on (with the use-package definition on `init.el') you should
just open .md and have the highlight and stuff.

--- end of the guide

>From 45796df36129ec77c532b3fa21cdd0b8033c9777 Mon Sep 17 00:00:00 2001
From: Rahul Martim Juliato <rahul.juliato@gmail.com>
Date: Fri, 19 Apr 2024 23:21:20 -0300
Subject: [PATCH] feat: add markdown-ts-mode

 * lisp/textmodes/markdown-ts-mode.el: New file.
 * doc/emacs/emacs.texi: Add Markdown to the manual
 * etc/NEWS: Announce markdown-ts-mode
---
 doc/emacs/emacs.texi               |   1 +
 etc/NEWS                           |   5 ++
 lisp/textmodes/markdown-ts-mode.el | 106 +++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)
 create mode 100644 lisp/textmodes/markdown-ts-mode.el

diff --git a/doc/emacs/emacs.texi b/doc/emacs/emacs.texi
index 7d77f13ab21..244c822ee04 100644
--- a/doc/emacs/emacs.texi
+++ b/doc/emacs/emacs.texi
@@ -618,6 +618,7 @@ Top
 * Enriched Text::       Editing text enriched with fonts, colors, etc.
 * Text Based Tables::   Commands for editing text-based tables.
 * Two-Column::          Splitting text columns into separate windows.
+* Markdown::            Major mode for editing Markdown files.
 
 Filling Text
 
diff --git a/etc/NEWS b/etc/NEWS
index 8ad1e78ca60..06fbaa03b55 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1688,6 +1688,11 @@ A major mode based on the tree-sitter library for 
editing Elixir files.
 *** New major mode 'lua-ts-mode'.
 A major mode based on the tree-sitter library for editing Lua files.
 
++++
+*** New major mode 'markdown-ts-mode'.
+A major mode based on the tree-sitter library for editing Markdown files.
+
+
 ** Minibuffer and Completions
 
 +++
diff --git a/lisp/textmodes/markdown-ts-mode.el 
b/lisp/textmodes/markdown-ts-mode.el
new file mode 100644
index 00000000000..063780a772f
--- /dev/null
+++ b/lisp/textmodes/markdown-ts-mode.el
@@ -0,0 +1,106 @@
+;;; markdown-ts-mode.el --- tree sitter support for Markdown  -*- 
lexical-binding: t; -*-
+
+;; Copyright (C) 2024 Free Software Foundation, Inc.
+
+;; Author     : Rahul Martim Juliato <rahul.juliato@gmail.com>
+;; Maintainer : Rahul Martim Juliato <rahul.juliato@gmail.com>
+;; Created    : April 2024
+;; Keywords   : markdown md languages tree-sitter
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+;;
+
+;;; Code:
+
+(require 'treesit)
+(require 'subr-x)
+
+(declare-function treesit-node-parent "treesit.c")
+(declare-function treesit-node-type "treesit.c")
+(declare-function treesit-parser-create "treesit.c")
+
+(defvar markdown-ts--treesit-settings
+  (treesit-font-lock-rules
+   :language 'markdown
+   :override t
+   :feature 'delimiter
+   '([ "[" "]" "(" ")" ] @shadow)
+
+   :language 'markdown
+   :feature 'paragraph
+   '([((atx_heading) @font-lock-keyword-face)
+      ((block_quote_marker) @font-lock-string-face)
+      ((code_span) @font-lock-string-face)
+      ((emphasis) @underline)
+      ((image_description) @link)
+      ((indented_code_block) @font-lock-string-face)
+      ((link_destination) @font-lock-string-face)
+      ((setext_heading) @font-lock-keyword-face)
+      ((strong_emphasis) @bold)
+      ((thematic_break) @shadow)
+      (block_quote (block_quote_marker) @font-lock-string-face)
+      (block_quote (paragraph) @font-lock-string-face)
+      (fenced_code_block (code_fence_content) @font-lock-string-face)
+      (fenced_code_block (fenced_code_block_delimiter) @font-lock-doc-face)
+      (inline_link (link_destination) @font-lock-string-face)
+      (inline_link (link_text) @link)
+      (list_item (list_marker_dot) @font-lock-keyword-face)
+      (list_item (list_marker_minus) @font-lock-keyword-face)
+      (list_item (list_marker_plus) @font-lock-keyword-face)
+      (list_item (list_marker_star) @font-lock-keyword-face)
+      (shortcut_link (link_text) @link)
+      ])))
+
+(defun markdown-ts-imenu-node-p (node)
+  "Check if NODE is a valid entry to imenu."
+  (equal (treesit-node-type (treesit-node-parent node))
+         "atx_heading"))
+
+(defun markdown-ts-imenu-name-function (node)
+  "Return an imenu entry if NODE is a valid header."
+  (let ((name (treesit-node-text node)))
+    (if (markdown-ts-imenu-node-p node)
+       (thread-first (treesit-node-parent node)(treesit-node-text))
+      name)))
+
+(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 text-mode "Markdown"
+  "Major mode for editing Markdown using tree-sitter grammar."
+  (setq-local font-lock-defaults nil
+             treesit-font-lock-feature-list '((delimiter)
+                                              (paragraph)))
+
+  (setq-local treesit-simple-imenu-settings
+              `(("Headings" markdown-ts-imenu-node-p nil 
markdown-ts-imenu-name-function)))
+
+  (when (treesit-ready-p 'markdown)
+    (treesit-parser-create 'markdown)
+    (markdown-ts-setup)))
+
+(derived-mode-add-parents 'markdown-ts-mode '(markdown-mode))
+
+(if (treesit-ready-p 'markdown)
+    (add-to-list 'auto-mode-alist '("\\.md\\'" . markdown-ts-mode)))
+
+(provide 'markdown-ts-mode)
+;;; markdown-ts-mode.el ends here
-- 
2.39.2




Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:

> 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... 
>
> 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 :)
>
> —
> Kind Regards
> Jostein Kjønigsen
>
>  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.
>
>  From 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                           |   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-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)))
>
>  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.