all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Philip Kaludercic <philipk@posteo.net>
To: Pranshu Sharma <sirsharmapranshu@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: New package: haskell-ts-mode
Date: Thu, 29 Aug 2024 14:41:54 +0000	[thread overview]
Message-ID: <87h6b3qtl9.fsf@posteo.net> (raw)
In-Reply-To: <CAFBmwwNk=3SJ0dgceTXRHPnbOH40TSWp-aDXpsts5MRPSL0J5g@mail.gmail.com> (Pranshu Sharma's message of "Thu, 29 Aug 2024 20:57:29 +1000")

Pranshu Sharma <sirsharmapranshu@gmail.com> writes:

> Oh well, haskell-tng-mode is gpl, and I wouldn't consider the contributions
> 'significant', but I think adding package to  NonGNU elpa would be best to
> avoid drama.

OK.

> One day we might find the mythical prof that teaches his students elisp,
> till then stay optimistic.

[ I have actually held small "Introduction to Emacs and Elisp" courses,
but only as a student. Either way, that is off-topic here. ]

> btw, I had to pull back your suggested changes to the run haskell function,
> they did not work

What specifically didn't work?  I only tested most of my proposals
superficially.

> On Thu, Aug 29, 2024 at 8:27 PM Philip Kaludercic <philipk@posteo.net>
> wrote:
>
>> Pranshu Sharma <sirsharmapranshu@gmail.com> writes:
>>
>> > Oh yeah, I just realised that and fixed the electric pair mode error.
>>
>> 1+
>>
>> > The seq-do part I did not code myself, I copied it from haskell-tng-mode,
>> > as stated in the comment.
>>
>> That reminds me, haskell-tng-mode is on NonGNU ELPA, but your patch
>> appears to be for GNU ELPA.  If you code is based on haskell-tng-mode,
>> then there might be licensing issues.  Should we add the package to
>> NonGNU ELPA instead, or can you attest to having no significant
>> contributions from anyone else?
>>
>> > Homework? students? do you teach your students elisp?
>>
>> Ugh, no sorry for the misunderstanding.  I was a TA for systems
>> programming and taught C.  My point was that LLMs tended to generate
>> unusual C code.  I just have weird sensibilities since then.
>>
>> > On Thu, Aug 29, 2024 at 7:07 PM Philip Kaludercic <philipk@posteo.net>
>> > wrote:
>> >
>> >> Pranshu Sharma <sirsharmapranshu@gmail.com> writes:
>> >>
>> >> > 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.
>> >>
>> >> OK, sorry about the last suggestion then, as I said, I am not familiar
>> >> with treesitter.
>> >>
>> >> > With the spacing, I did what you suggested then used emacs to indent
>> the
>> >> > whole page, and added elpaignore file
>> >>
>> >> 1+
>> >>
>> >> Note that `electric-pair-pairs' is now malconstructed.  It is a list of
>> >> lists, where each list begins with the symbol `cons'.  Not a list of
>> >> cons-cells holding two characters.
>> >>
>> >> > Also no, I did not use Ai, what made you think so?
>> >>
>> >> There were a some unusual constructs repeated a number of times that I
>> >> have never seen before (the way you used seq-do was the main example).
>> >> It reminded me of homework submissions I have seen students submit that
>> >> /were/ AI generated.
>> >>
>> >> >
>> >> > 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
>> >> >>
>> >>
>> >> --
>> >>         Philip Kaludercic on peregrine
>> >>
>>
>> --
>>         Philip Kaludercic on peregrine
>>

-- 
	Philip Kaludercic on peregrine



  reply	other threads:[~2024-08-29 14:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-28  6:07 New package: haskell-ts-mode Pranshu Sharma
2024-08-28 15:18 ` Philip Kaludercic
2024-08-29  7:24   ` Pranshu Sharma
2024-08-29  9:07     ` Philip Kaludercic
2024-08-29  9:29       ` Pranshu Sharma
2024-08-29 10:26         ` Philip Kaludercic
2024-08-29 10:57           ` Pranshu Sharma
2024-08-29 14:41             ` Philip Kaludercic [this message]
2024-09-01  5:33 ` Pranshu Sharma
2024-09-01 20:04   ` Philip Kaludercic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6b3qtl9.fsf@posteo.net \
    --to=philipk@posteo.net \
    --cc=emacs-devel@gnu.org \
    --cc=sirsharmapranshu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.