This repository was archived by the owner on Mar 29, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1
Cloogle QuickPick #12
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To determine the
/src
URL you need to usegeneralData.filename
instead ofgeneralData.modul
, to account for modules like ParsersTestware in Platform that are not in the library root.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the long run it could make sense to create a JS library for the interface in which things like this can be abstracted. If the web frontend uses it as well it stays maintained. And this would also make it easier to add search functionality to the Atom plugin and the Monaco editor in the Clean sandbox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good idea! For now I was just in a little bit of a hurry to get the extension back to a place where it would be useful again. Eventually we should abstract all of this in a language server (maybe even written in Clean?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would make it harder to share sources with the web frontend, but has obvious other advantages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@camilstaps I agree with you about having an actual, separate Cloogle JS library.
It has been a while I haven't played with Clean (nor with typescript actually!), and got a bit of nostalgia this weekend... so I ended up writing https://github.com/W95Psp/cloogle-js today!
It has exhaustive type definitions "scrapped" from the Cloogle's API (see the --rather dirty--
script/extract.js
file)!I also make use of a library (io-ts) to validate incoming data.
Made a few tests (see
examples/Tests.ts
), it seems alright.I can add both of you (@camilstaps @ErinvanderVeen) as maintainer of the repo if you like, I won't have much time to maintain it.
Concerning the (potential) language server, I was wondering: has Clean a REPL or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, I'm happily surprised! I have never worked with TypeScript. It will take some time to see how best to integrate this in the web frontend. But having this functionality in a separate library is clearly an important step in the right direction.
As far as I'm aware, the closest thing to a REPL would be iClean. This is nothing fancy; it keeps track of your definitions, writes a module to a file and has the compiler compile it in the background. The user is responsible for indicating which lines that she enters are to be kept (function definitions) and which are to be evaluated (Start rules). Because of the setup the whole module is recompiled on every iteration. It's really quite useless, and will probably not compile with the current Clean builds.
I would be interested in keeping track of the definitions by importing the compiler, compiling only the modified code / Start expression, and dynamically linking the generated ABC code. (If you'd use the ABC interpreter for that, dynamic linking becomes quite easy and you can bypass the code generator.) As I recall, the main problem however is the user interface. Functions can have multiple alternatives and you need to give the user control to reorder them. This is a far less interesting problem, to me at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool :)
You can use the library directly as a Javascript library if you like.
If you need any help, don't hesitate, I might have some time on weekends!
Btw, I was not able to
git clone https://gitlab.science.ru.nl/cloogle/cloogle-org && cd cloogle-org && touch cloogle.log && sudo docker-compose up
.`make -C Cloogle compiler` fails with this error log
Nice, I did not know iClean existed! I thought about such an approach, but indeed, this would be quickly useless for an LSP, since recompiling the whole current module would lead to an important latency for the end-user.
If I understand you correctly,
clm someModule
normally produces a binary, but can just produce ABC intermediate code, that can be interpreted, right?For a language server protocol, we would not even need to produce intermediate code, we just need to compute type signature or collect errors otherwise. I'm aware of the
-lt
flag forclm
, but isn't there a way to plug into Clean's compiler to do so? I have never looked into Clean's compiler, I don't even know if it is bootstrapped in Clean or not (plain C?).Anyway, having a (fast!) way to output the type and the dependencies for the top levels of a module would be sufficient to build a nice enough and reactive enough language protocol server I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably need to do
git clone --recursive
(orgit submodule update --init --recursive
after cloning) to fetch the submodules. I'll clarify this in the readme, thanks.For an LSP server iClean is entirely useless, yes, but you don't need a REPL at all, right? Cloogle plugs in to the compiler to get type information, and cloogle-tags builds on cloogle to output a ctags file. That could be the base for an LSP server, and you would have to reindex modules when they are modified. (I believe
-lt
prints the inferred types, not the specified types, and you want the specified types - but I may be wrong.) Cloogle-tags is sufficiently fast; it takes perhaps a few seconds on the entirelib
directory so to reindex a single module is very fast.To answer your questions: the Clean compiler has a C backend, and can be bootstrapped with the ABC code for the Clean frontend, because the code generator is written in C. It always produces ABC code in Clean System Files, which is converted to machine code by the code generator. In recent years the possibility has been added to generate byte code from (optimized) ABC code and interpret it, either natively or in WebAssembly. I would try to use this interpreter to build a REPL if I were to make one. You can see https://gitlab.science.ru.nl/clean-and-itasks/abc-interpreter for details or read the paper, but as you observed it is not that relevant for an LSP server.
On a tangent, if someone were to build an LSP server, it may be a good idea to depend on an existing SDK. The obvious choice would be the MS library for node.js. Then you could run the Clean things, ultimately depending on the compiler, in WebAssembly; this is known to work. This comes with a performance penalty though, so alternatively a C++ SDK could be used or the Clean things could run natively in a separate process.
This all seems to require quite a bit of work. For the time being just using the ctags file generated by cloogle-tags in VSCode seems like the easiest solution. Perhaps cloogle-tags could be extended to be able to reindex certain files only, and this plugin could check after compiling a project which modules have been changed and reindex those.
Please don't feel obliged to do any of these things. I'm just writing down some ideas in case someone wants to have a go at it, but it looks like a lot of work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, sorry to barge in here. are there any updates about this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, sorry, forgot about that!
That definitely can be merged!
And thanks @camilstaps for the explanation, that's indeed an interesting discussion for someone willing to implement this kind of feature :)