Re: New package: haskell-ts-mode

[Pranshu Sharma <sirsharmapranshu@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 10926 bytes --]

Hello Phlip,

Thanks for the edits, I applied all of it apart from when when you want
defun changed to macro, that does not work, as closures are not properly
supported by treesit.

With the spacing, I did what you suggested then used emacs to indent the
whole page, and added elpaignore file

Also no, I did not use Ai, what made you think so?


On Thu, Aug 29, 2024 at 1:18 AM Philip Kaludercic <philipk@posteo.net>
wrote:

> Pranshu Sharma <sirsharmapranshu@gmail.com> writes:
>
> > Hello all
> >
> > haskell-ts-mode is a major mode for haskell that uses treesitter to
> provide
> > features such as indentation and colouring. The mode can be found at
> > pranshu/haskell-ts-mode:
> > A haskelll mode that uses treesitter - Codeberg.org
> > <https://codeberg.org/pranshu/haskell-ts-mode>.
>
> First of all, the spacing and indentation is inconsistent.  Please
> address that, and perhaps add a directory local option to enforce
> indentation with spaces to ensure a consistent look.
>
> I cannot comment on the tree-sitter stuff since I don't really
> understand it, but otherwise I have a few comments that I would like you
> to consider:
>
> diff --git a/haskell-ts-mode.el b/haskell-ts-mode.el
> index 4ba7f42..391db66 100644
> --- a/haskell-ts-mode.el
> +++ b/haskell-ts-mode.el
> @@ -2,7 +2,6 @@
>
>  ;; Copyright (C) 2024  Pranshu Sharma
>
> -
>  ;; Author: Pranshu Sharma <pranshusharma366 at gmail>
>  ;; URL: https://codeberg.org/pranshu/haskell-ts-mode
>  ;; Package-Requires: ((emacs "29.3"))
> @@ -23,6 +22,7 @@
>  ;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
>
>  ;;; Commentary:
> +
>  ;; This is a WIP mode that uses treesitter to provide all the basic
>  ;; major mode stuff, like indentation, font lock, etc...
>
> @@ -31,6 +31,7 @@
>  (require 'comint)
>  (require 'treesit)
>
> +;; From what I understand this is necessary for tree-sitter modes, but
> can you at least also add the ARGLIST argument.  Also, some functions such
> as `treesit-induce-sparse-tree' appear not to be used?
>  (declare-function treesit-parser-create "treesit.c")
>  (declare-function treesit-induce-sparse-tree "treesit.c")
>  (declare-function treesit-node-child "treesit.c")
> @@ -46,10 +47,11 @@
>      (otherwise signature)))
>
>  (defvar haskell-ts-use-indent t
> -  "Set to nil if you don't want to use Emacs indent.")
> +  "Set to nil if you don't want to use Emacs indent.") ;rephrase this
> would the double negation
>
>  (defvar haskell-ts-font-lock-level 4
>    "Level of font lock, 1 for minimum highlghting and 4 for maximum.")
> +;; both of these variables are actually user options and should be
> declared using `'defcustom'!
>
>  (defvar haskell-ts-prettify-symbols-alits
>        '(("\\" . "λ")
> @@ -60,6 +62,7 @@
>         ("<=" . "≥")
>         (">=" . "≤")))
>
> +;; Checkdoc complains that
>  (defun haskell-ts-font-lock ()
>      (treesit-font-lock-rules
>         :language 'haskell
> @@ -157,7 +160,7 @@
>            ((parent-is "apply") parent -1)
>            ((node-is "quasiquote") grand-parent 2)
>            ((parent-is "quasiquote_body") (lambda (_ _ c) c) 0)
> -          ;; Do Hg
> +          ;; Do Hg  ;; what does "Hg" mean?
>            ((lambda (node parent bol)
>               (let ((n (treesit-node-prev-sibling node)))
>                 (while (string= "comment" (treesit-node-type n))
> @@ -232,6 +235,7 @@
>
>  ;; Copied from haskell-tng-mode, changed a bit
>  (defvar haskell-ts-mode-syntax-table
> +  ;; Should this be wrapped in an `eval-when-compile'?
>        (let ((table (make-syntax-table)))
>         (map-char-table
>          (lambda (k v)
> @@ -241,7 +245,7 @@
>                 (modify-syntax-entry k "_" table))))
>          (char-table-parent table))
>         ;; whitechar
> -       (seq-do
> +       (seq-do                         ;seq-do has a relatively high
> overhead, try to avoid it
>          (lambda (it) (modify-syntax-entry it " " table))
>          (string-to-list "\r\n\f\v \t"))
>         ;; ascSymbol
> @@ -268,46 +272,40 @@
>         (modify-syntax-entry ?\{  "(}1nb" table)
>         (modify-syntax-entry ?\}  "){4nb" table)
>         (modify-syntax-entry ?-  "_ 123" table) ;; TODO --> is not a
> comment
> -       (seq-do
> -        (lambda (it) (modify-syntax-entry it ">" table))
> -        (string-to-list "\r\n\f\v"))
> +       (dolist (c (string-to-list "\r\n\f\v")) ;though unrolling wouldn't
> be bad either
> +         (modify-syntax-entry c ">" table))
> +
>         table))
>
> +(defun haskell-ts-imenu-name-function (check-func)
> +  (lambda (node)
> +    (if (funcall check-func node)
> +       (haskell-ts-defun-name node)
> +      nil)))
>
> -(defmacro haskell-ts-imenu-name-function (check-func)
> -  `(lambda (node)
> -      (if (funcall ,check-func node)
> -         (haskell-ts-defun-name node)
> -       nil)))
> -
> -(defun haskell-ts-indent-para()
> +(defun haskell-ts-indent-para ()
>    "Indent the current paragraph."
>    (interactive)
> -  (save-excursion
> -    (backward-paragraph)
> -    (let ((p (point)))
> -      (forward-paragraph)
> -      (indent-region p (point)))))
> +  (when-let ((par (bounds-of-thing-at-point 'paragraph)))
> +    (indent-region (car par) (cdr par))))
>
>  (defvar haskell-ts-mode-map
>    (let ((km (make-sparse-keymap)))
>      (define-key km (kbd "C-c C-c") 'haskell-ts-compile-region-and-go)
>      (define-key km (kbd "C-c C-r") 'haskell-ts-run-haskell)
> -    (define-key km (kbd "C-M-q") 'haskell-ts-indent-para)
> +    (define-key km (kbd "C-M-q") 'haskell-ts-indent-para) ;is this
> necessary when `prog-fill-reindent-defun' is bound to M-q?
>      km)
> -  "Map for haskell-ts-mode")
> +  "Map for haskell-ts-mode.")
>
>  ;;;###autoload
>  (define-derived-mode haskell-ts-mode prog-mode "haskell ts mode"
>    "Major mode for Haskell files using tree-sitter."
> -  :syntax-table haskell-ts-mode-syntax-table
> -  :interactive t
>    (unless (treesit-ready-p 'haskell)
>      (error "Tree-sitter for Haskell is not available"))
>    (treesit-parser-create 'haskell)
>    (setq-local treesit-defun-type-regexp
> "\\(?:\\(?:function\\|struct\\)_definition\\)")
>    ;; Indent
> -  (and haskell-ts-use-indent
> +  (and haskell-ts-use-indent           ;do you mean `when'?
>         (setq-local treesit-simple-indent-rules haskell-ts-indent-rules)
>         (setq-local indent-tabs-mode nil))
>    ;; Comment
> @@ -316,21 +314,21 @@
>    (setq-local comment-start-skip "\\(?: \\|^\\)-+")
>    ;; Elecric
>    (setq-local electric-pair-pairs
> -             (list (cons ?` ?`) (cons ?\( ?\)) (cons ?{ ?}) (cons ?\"
> ?\") (cons ?\[ ?\])))
> +             '((?` . ?`) (?\( . ?\)) (?{ . ?}) (?\" . ?\") (?\[ . ?\])))
>    ;; Nav
> -  (setq-local treesit-defun-name-function 'haskell-ts-defun-name)
> +  (setq-local treesit-defun-name-function #'haskell-ts-defun-name)
>    (setq-local treesit-defun-type-regexp "function")
>    (setq-local prettify-symbols-alist haskell-ts-prettify-symbols-alits)
>    ;; Imenu
>    (setq-local treesit-simple-imenu-settings
>               `((nil haskell-ts-imenu-func-node-p nil
> -                    ,(haskell-ts-imenu-name-function
> 'haskell-ts-imenu-func-node-p))
> +                    ,(haskell-ts-imenu-name-function
> #'haskell-ts-imenu-func-node-p))
>                 ("Signatures.." haskell-ts-imenu-sig-node-p nil
> -                ,(haskell-ts-imenu-name-function
> 'haskell-ts-imenu-sig-node-p))
> +                ,(haskell-ts-imenu-name-function
> #'haskell-ts-imenu-sig-node-p))
>                 ("Data..." haskell-ts-imenu-data-type-p nil
>                  (lambda (node)
>                    (treesit-node-text (treesit-node-child node 1))))))
> -  ;; font-lock.
> +  ;; font-lock
>    (setq-local treesit-font-lock-level haskell-ts-font-lock-level)
>    (setq-local treesit-font-lock-settings (haskell-ts-font-lock))
>    (setq-local treesit-font-lock-feature-list
> @@ -379,23 +377,32 @@
>
>  (defun haskell-ts-run-haskell()
>    (interactive)
> -  (when (not (comint-check-proc "*haskell*"))
> -    (set-buffer (apply (function make-comint)
> -                      "haskell" "ghci" nil `(,buffer-file-name))))
> -  (pop-to-buffer-same-window "*haskell*"))
> +  (pop-to-buffer-same-window           ;really in the same window?
> +   (or
> +    (comint-check-proc "*haskell*")
> +    (make-comint "*haskell* repl" "ghci" nil buffer-file-name))))
> +
>
>  (defun haskell-ts-haskell-session ()
>    (get-buffer-process "*haskell*"))
>
> +(defvar eglot-server-programs)         ;to avoid the byte-compiler error
>  (defun haskell-ts-setup-eglot()
>    (when (featurep 'eglot)
> -   (add-to-list 'eglot-server-programs
> +    ;; Eglot was added to the core along with tree-siter, so there is
> +    ;; no case where tree-sitter is available, but eglot is not.
> +    (add-to-list 'eglot-server-programs
>                '(haskell-ts-mode . ("haskell-language-server-wrapper"
> "--lsp")))))
>
> +;; Note that you write (eval-when-load 'eglot
> +;; (haskell-ts-setup-eglot)) in README.org, but that doesn't do what
> +;; you want it to.  That will modify eglot-server-programs and
> +;; evaluate the result when eglot is loaded.  You presumably wanted to
> +;; use `with-eval-after-load'?
> +
>  (when (treesit-ready-p 'haskell)
>    (add-to-list 'auto-mode-alist '("\\.hs\\'" . haskell-ts-mode)))
>
>  (provide 'haskell-ts-mode)
>
>  ;;; haskell-ts-mode.el ends here
> -
>
> (Also, a general question, do you use LLMs to generate some of the code?)
>
> >
> > The attached patch is for the elpa repo
> > Thanks,
> > Pranshu
> > diff --git a/elpa-packages b/elpa-packages
> > index 137fef0348..451a67d395 100644
> > --- a/elpa-packages
> > +++ b/elpa-packages
> > @@ -368,6 +368,9 @@
> >   (gtags-mode         :url "https://github.com/Ergus/gtags-mode")
> >   (guess-language     :url "
> https://github.com/tmalsburg/guess-language.el"
> >    :merge t)
> > + (haskell-ts-mode       :url "
> https://codeberg.org/pranshu/haskell-ts-mode"
> > +  :doc "README.org"
>
> Are you sure that you want the README to be installed as a manual?
>
> > +  :ignored-files ("*.png" "LICENSE"))
>
> You can list the files you wish to ignore in an .elpaignore file in your
> repository; its preferable to using :ignored-files as it is more
> flexible and easier to adjust if something changes on your end.
>
> >   (hcel                       :url "https://g.ypei.me/hc.el.git")
> >   (heap                       :url nil) ;"
> http://www.dr-qubit.org/git/predictive.git"
> >   (hiddenquote                :url "
> https://gitlab.com/mauroaranda/hiddenquote/hiddenquote")
> >
>
> --
>         Philip Kaludercic on peregrine
>

[-- Attachment #2: Type: text/html, Size: 14126 bytes --]