emacs-devel
[Top][All Lists]
Advanced

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

Re: Merge haskell-ts-mode in upstream


From: Philip Kaludercic
Subject: Re: Merge haskell-ts-mode in upstream
Date: Sat, 28 Dec 2024 16:15:10 +0000

Pranshu Sharma <pranshu@bauherren.ovh> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> I see that you have started a new repository.  Do you want us to mirror
>> your changes with all the commit history, or are you OK with us just
>> copying over the coded periodically whenever you want to update the code?
>
> Ok, it seems like I'll be sticking to git one anyway, since
> git-hg-bridge or something doesn't work.  I am fine with using the old
> repo.

Wait, I am confused.  The initial proposal was to add the package to
emacs.git, right?

>>>   :type 'boolean)
>>>
>>> (defcustom haskell-ts-font-lock-level 4
>>>   "Level of font lock, 1 for minimum highlghting and 4 for maximum."
>>>   :group 'haskell-ts-mode
>>>   :type 'integer)
>>
>> It might even make sense to use a choice here, as not all integer values
>> are valid.
>
> From the customize manual, using the :options is only avialiable with
> few :types, like hook and plist.  I don't think it works for numbers.

You could just use something of the form

 (choice (const :tag "Minimal Highlighting" 1) ...)

>> You should clean up the indentation, you seem to have tabs in the code
>> made the code intended in a harder to read way on my machine.
>
> Ok, I changed the tab width and indented whole page.  tell me if still a
> problem.

There still seem to be a few instances.  You can use whitespace-mode to
highlight them ICYDK.

>>>
>>> (defun haskell-ts-indent-defun (pos)
>>>   "Indent the current function."
>>>   (interactive "d")
>>>   (let ((node (treesit-node-at pos)))
>>>     (while (not (string-match
>>>              "^declarations$\\|haskell"
>>>              (treesit-node-type (treesit-node-parent node))))
>>>       (setq node (treesit-node-parent node)))
>>>     (indent-region (treesit-node-start node) (treesit-node-end node))))
>>
>> Why is this function necessary, if we already have the general commands
>> for indenting a defun?  If it is, it should probably be explained in the
>> docstring.
>
> Haskell functions are not paren wrapped(opptionally they are), so when I
> tested those functions don't work. C-M-h works, but the indentation is a
> little different in treesitter based mode, it works differently in
> incomplete sentences.  Meaning newline-indent would rarley be the final
> indentation of any expression.  I think the reason is too techical to
> include in docstring.

So regular C-M-h would just re-indent a single equation, while this
matches all equations that constitute a total definition?

>>> (defun haskell-ts-compile-region-and-go (start end)
>>>   "Compile the text from START to END in the haskell proc."
>>>   (interactive "r")
>>>   (let ((hs (haskell-ts-haskell-session)))
>>>     (comint-send-string hs ":{\n")
>>>     (comint-send-region hs start end)
>>
>> You should probably do something to ensure that the interval doesn't
>> contain ":}"?
>
> I don't get what you mean.

If you select a region in the buffer that would happen to include a

--8<---------------cut here---------------start------------->8---
:}
--8<---------------cut here---------------end--------------->8---

on one line, could this mess up the state of GHCi?

>>>     (comint-send-string hs "\n:}\n")))
>>>
>>> (defun haskell-ts-run-haskell()
>>>   (interactive)
>>>   (pop-to-buffer-same-window
>>>    (if (comint-check-proc "*haskell*")
>>>        "*haskell*"
>>>      (make-comint "haskell" "ghci" nil buffer-file-name))))
>>
>> 1. Would it make sense to have a user option that allows customising
>>    this, in case someone uses Cabal, Stack or whatever else?
>
> I have a lot to say about this: Haskell's eco system is something that
> will have you scratching your head when sleeping.  Trying to support
> these utilities is soft suacide.  This package provides a simple ghci
> wrapper, and I don't want it to become too complex, and inturn hard to
> maintain(these tools are diddy when it comes to backward compatibity)
> and understand.  If someone wants to make a complex ghci wrapper which
> can do all this, they could(but shouldn't, for the sake of mental
> health).  This mode attempts ot provide syntax+indentation+imenu+
> someminorstuff *fullstop*, not provide the whole hassle-less haskell
> development experiance(even if it existed).  The current model is that
> people just do `import`in ghc. iirc, no fancy cabal/stakc program exists
> to start ghci with all the app loaded.  To haskell-ts-mode, haskell goes
> as far as ghc, cabal and stack don't exist.

I don't have much experience with these tools, all I know is that there
are commands like "stack ghci" that might be something some people could
be interested in setting.

> Wait, now that I read it, you meant an option to specify ghci
> path(added).  I went on a rant as soon as I saw cabal, oh well...

I guess that should be fine as well.

>> 2. The regular haskell-mode has a `run-haskell' command that creates the
>>    same buffer name.  Assuming that it is not that unusual for people to
>>    have both installed, when trying out the one or the other, shouldn't
>>    you take some precaution to detect that scenario?
>
> yes, that makes sense, I changed buffer name to *Inferior Haskell*.
> Initially I wanted to name the functino run-haskell for consistancy(eg
> run-php (in php-ts), run-scheme, etc...), but this is fine.

If we move the code into the core, then I think it would be fair to add
such a function.  We shouldn't have to inconvenience users because of
third-party packages on NonGNU ELPA.

Also, you seem to have named the buffer "*Inferior haskell*" (lower
case) instead of "*Inferior Haskell*" in `haskell-ts-haskell-session'.
Perhaps you can pull out the string into a constant.

> I attach new file, I have also pushed changes to codebrg repo.

I think it would be someone with more Tree Sitter experience could take
a look at the code as well.  Perhaps we should make this more formal by
moving over to the bug tracker?



reply via email to

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