Re: Merge haskell-ts-mode in upstream

[Pranshu Sharma via "Emacs development discussions." <emacs-devel@gnu.org>]

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

Philip Kaludercic <philipk@posteo.net> writes:

> 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?

Yes, but iiuc it will still have own repo and emacs will just mirror it
or smth.

>>>>   :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) ...)
>

Oh, that was simple...

>>> 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.

('ICYDK' means "In case you didn't know", 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?

Not really, it is mostly convince for C-M-h TAB, but since it is meant ot be
used a lot, it get it's own special binding.  The rules are tiny bit
different.

>>>> (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
>
> :}
>
> on one line, could this mess up the state of GHCi?

Turns out it does, the only time this is valid suntax is in quasiquote
and multiline comment.  I kinda expected ghci to be smart enough to
handle it, but I did it now

>>>>     (comint-send-string hs "\n:}\n")))
>>>>

[37 lines omitted]
>>
>> 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.

I agree with this.

> 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.

Done

>> 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?

Sounds good.


[-- Attachment #2: haskell-ts-mode.el --]
[-- Type: application/emacs-lisp, Size: 16052 bytes --]

[-- Attachment #3: Type: text/plain, Size: 47 bytes --]



-- 
Pranshu Sharma <https://p.bauherren.ovh>