emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jostein Kjønigsen
Subject: Re: [PATCH] feat: add markdown-ts-mode
Date: Sat, 20 Apr 2024 19:45:05 +0200

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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]