Add Janet Lexer #2557
Add Janet Lexer #2557
Conversation
sogaiu
force-pushed
the
janet
branch
2 times, most recently
from
505013f
to
fe9e5cb
Compare
|
The most recent force-push was for updating handling of radix-based numbers. Number handling should be more uniform across decimal, hex, and radix now. |
|
I removed the use of an f-string and pushed that change. |
|
BTW, I've not had luck getting Here is part of the invocation output that seems to result in erroring out: That output looks related to this line. The version of tox here appears to be 4.11.3 and it looks to me like in that version, Similarly for pip, the version of pip here appears to be 23.2.1 and it looks to me like in that version, Or may be I am just confused...in any case, any hints about how I might get |
|
I made some modifications to I got further error output that looks like: I took a look at those lines and what I found for pluggy 1.3.0 was: # if self._name2plugin[name] == None registration was blocked: ignoreand for pip 23.2.1 was: # This should never happen if self.block == TrueThese lines do not look like they are active to me (comment / commented?). I tried disabling the associated checking in |
|
You apparently have a virtual environment |
|
Thanks a lot for taking a look and the explanation.
It seems that adding |
|
You don't need a venv. The whole point of tox is that it manages venvs for you. |
|
Ah, indeed? I had no idea. That sounds nice. Thanks for the explanation. |
|
Perhaps I would need to do something else then as there is no Perhaps this is because of the way python is packaged for my distribution. |
|
As a rule, you should use your distro exclusively to install things into the global environment, and use virtual environments for everything else, possibly through a tool like tox or pipx. Never ever use pip outside of a virtual environment unless you know very well what you're doing. (Depending on your distro, it might already give an error to protect you, if it has already adopted a standard known as PEP 668). There is inconsistent advice about this in the wild because the situation was confusing and bad for a long time, but it's clarified now. In the case of tox, simply try installing the |
|
It's nice to hear things have been clarified, thanks for mentioning it. The last time I used Python in any significant capacity I don't remember it being that way. Working with multiple projects also means that one might want a different version of a tool per project for reliable / reproducible results, so I try to avoid installing certain tools globally. |
|
Thanks a lot for the review and suggestions. I think I've responded to all comments and made attempts to address each item (except the Unicode identifier bits for which I gave my current thoughts). (I've also renamed |
|
Uh ok. I was just about to merge this. What's the reason for closing this? We'd obviously like to have a Janet lexer in the next release. |
|
Ah sorry, there is a discussion starting here that gives background. To give a little bit more information, we're using a hosted Zulip setup (which uses pygments AFAICT) for Janet community discussions and (AFAIK) we have no fine-grained control over how our pygments setup would behave (i.e. I don't think we can install a plugin to get alternate behavior for Janet syntax highlighting). If pygments ends up with a Janet lexer with the requested changes to identifiers to include most of Unicode and we specify using Janet syntax highlighting, I believe (non-ASCII containing) Unicode identifiers will end up being shown as acceptable with no warning (see this comment for some of my concerns). At this time, I do not feel comfortable being partially responsible for that kind of situation. I didn't realize this might happen before submitting the initial PR. I realize the pygments project has its own priorities and can appreciate the comment here. Thus, I thought it would be better to hold off until the situation is better understood (or we find some way to customize our highlighting). |
|
After discussing it we are OK with merging this in its current form. Thus I will reopen it and merge it. However, the 2.17 release was done in the meantime and there needs to be a bugfix release, so please allow a few days before the merge. |
|
Thanks for the consideration and explanation. |
|
Thank you for your contribution. |
This PR is an attempt at adding a lexer for Janet.
There was an earlier attempt, PR #1519 by uvtc [1], but the current PR is mostly an independent work.
Snippet test content was adapted from the custom tests for tree-sitter-janet-simple.
Result of running
toxlocally gave roughly:Cc: @pyrmont, @uvtc
[1] The earlier PR was studied however, and thus I've included uvtc in the AUTHORS file (along with pyrmont who helped with this PR).