unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Philip Kaludercic <philipk@posteo.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: ruijie@netyu.xyz,  casouri@gmail.com,  emacs-devel@gnu.org
Subject: Re: Code quality of some -ts-mode major modes
Date: Fri, 17 Mar 2023 15:49:15 +0000	[thread overview]
Message-ID: <87h6ujjs3o.fsf@posteo.net> (raw)
In-Reply-To: <835yaze6o6.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 17 Mar 2023 17:31:05 +0200")

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: ruijie@netyu.xyz,  casouri@gmail.com,  emacs-devel@gnu.org
>> Date: Fri, 17 Mar 2023 15:20:49 +0000
>> 
>> >> The rule-of-thumb that I go by is that `if' is used if you have two
>> >> cases you are interested in, especially if you are interested in the
>> >> return value, while `when' is more "imperative" in style and indicates
>> >> to the reader that the code is being executed for a side-effect.
>> >
>> > That is your personal preference.  Objectively, there's nothing wrong
>> > with using 'if' that has no 'else' part.  So changing someone's code
>> > to use 'when' where 'if' can do, or vice versa -- replacing 'when'
>> > with a single sexp in the body with 'if' -- has no real justification.
>> 
>> Technically no, but I do hope not to be mistaken that there is a
>> convention (along the lines I gave above) here that goes beyond just my
>> personal preference.  CLTL even says[p. 166]:
>
> It is fine for you to prefer this convention, but we don't mandate it
> in Emacs.

Ok, in that case I was mistaken.  I have just seen a number of commits
that either just or also made this change, making it seem like
established knowledge.

>> > Because if we add that to the code, we will need to maintain that for
>> > the observable future to be correct.  Comments, even if they are
>> > outdated, don't need such level of maintenance.  
>> 
>> That could be resolved by either pinning a revision or instead of
>> cloning the repository to download a tarball of a tag.  In fact that
>> should make the system even more stable than the way I see it being
>> promoted around the web currently, that just maps languages to Git
>> repository URLs.
>
> It can be resolved in more than one way, but all of them mean
> additional maintenance burden, so I don't think we should undertake
> that.

If you think so, but at least we agree that one reference to a grammar
would be helpful.

>> >                                                  Moreover, the fact
>> > that a given grammar was used for testing doesn't mean another grammar
>> > will not work as well.
>> 
>> I don't know of any language with multiple independent implementations
>
> I do.  Indeed, most have just one.  But not all.

Could you give me an example that I could try out?

>> > Again, I explained the rationale many times here.  I can explain
>> > again, but is that really necessary?
>> 
>> You had previously said that you are opposed to raising an error (or am
>> I mistaken?), while the above comment says "we want this to signal and
>> error".
>
> No you are mistaken.  We do want this to signal an error if
> tree-sitter is not compiled in or the grammar is not available.

But it doesn't, or am I completely oblivious that

  (treesit-ready-p 'something-i-just-made-up)

just displays a warning?  The way I am currently reading/seeing the code
is that if tree-sitter and the grammar are available, then it will be
initialised but otherwise we just get a warning and a pretty plain mode.

>> > Help in reviewing patches when they are posted is also very welcome.
>> > It takes more than one pair of eyes to spot every bit that needs
>> > attention.
>> 
>> I'll try and do so when I notice one.  I have also been sketching out
>> support for a markdown-ts-mode to better understand the how tree-sitter
>> works, which could help.
>
> TIA.

-- 
Philip Kaludercic



  reply	other threads:[~2023-03-17 15:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-17 10:08 Code quality of some -ts-mode major modes Philip Kaludercic
2023-03-17 10:29 ` Ruijie Yu via Emacs development discussions.
2023-03-17 11:52   ` Eli Zaretskii
2023-03-17 12:37   ` Philip Kaludercic
2023-03-17 13:54     ` Eli Zaretskii
2023-03-17 15:20       ` Philip Kaludercic
2023-03-17 15:31         ` Eli Zaretskii
2023-03-17 15:49           ` Philip Kaludercic [this message]
2023-03-17 16:35             ` Eli Zaretskii
2023-03-17 16:53               ` Philip Kaludercic
2023-03-17 21:45       ` Dmitry Gutov
2023-03-18  5:59         ` Eli Zaretskii
2023-03-17 11:46 ` Eli Zaretskii

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

  List information: https://www.gnu.org/software/emacs/

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

  git send-email \
    --in-reply-to=87h6ujjs3o.fsf@posteo.net \
    --to=philipk@posteo.net \
    --cc=casouri@gmail.com \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=ruijie@netyu.xyz \
    /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 public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).