Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added capabilities file #158

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Parsifal-M
Copy link

@Parsifal-M Parsifal-M commented Dec 1, 2022

Hey @anderseknert! 👋

Added the capabilities file 😄

Thank you for letting me work on this 👍

Signed-off-by: Peter Macdonald macdonald.peter90@gmail.com

Signed-off-by: Peter Macdonald <macdonald.peter90@gmail.com>
@anderseknert
Copy link
Collaborator

Good evening! And sorry for not getting back to you before... hectic weekend 👩‍👩‍👧‍👦 😅
The file looks good to me! I would however have expected some script or something to be part of this PR — i.e. something that takes the capabilities.json from OPA and filters it based on the capabilities provided by Jarl — bonus points for both clj and cljs.

@Parsifal-M
Copy link
Author

Good evening! And sorry for not getting back to you before... hectic weekend 👩‍👩‍👧‍👦 😅
The file looks good to me! I would however have expected some script or something to be part of this PR — i.e. something that takes the capabilities.json from OPA and filters it based on the capabilities provided by Jarl — bonus points for both clj and cljs.

Yeah I can do that! Let me see if I can make something a bit more tidy, was a bit hack and slash to put it together 😂

@anderseknert
Copy link
Collaborator

No worries! If it's too messy we'll work on it together :)

@johanfylling
Copy link
Collaborator

Awesome to have you here, helping out @Parsifal-M 😃!

Parsifal-M and others added 2 commits December 12, 2022 13:07
Signed-off-by: Peter Macdonald <macdonald.peter90@gmail.com>
@Parsifal-M
Copy link
Author

Hey! 👋

So I did a bit more hacking this weekend while I was sick, appologies if there is some funky stuff in there, I had a bit of brain fog 😆

Also, not sure what the .clj-kondo directory is 🤔 or the .lsp one... hoping you know more 😄

The script does not work yet I get this error:

Execution error at jarl.core/eval1788 (form-init5461747821624458434.clj:1).
Invalid token: :

Not sure if it's something to do with how the data looks like in supported or the json file itself.

Copy link
Collaborator

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Left some comments, but this is a good first attempt.

About the .lsp abd .clj-kondo directories, I think they're added by the VS Code plugin. Those directories only make sense in the core subdirectory, so should probably be added to gitignore here. You can delete them from the PR.

@@ -0,0 +1,20 @@
(ns jarl.gencaps
(:require [jarl.builtins.registry :as registry]
[clojure.core :as core]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required implicitly, and does not need to be here 🙂


;; Import capabilities.json as a Clojure data structure
(def capabilities (-> "capabilities.json"
core/slurp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, you don't need to state "core" anywhere

(def capabilities (-> "capabilities.json"
core/slurp
(core/str)
core/read-string))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error you're seeing is from this line — the read-string function is for Clojure data, not JSON. You can require jarl.encoding.json :as json and then use json/read-str here instead 🙂

(def supported (set (keys registry/builtins)))

;; Import capabilities.json as a Clojure data structure
(def capabilities (-> "capabilities.json"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the use of the threading macro! 😃

capabilities))

;; Write the matching JSON objects to a new file called gencaps.json
(core/spit "gencaps.json" (pr-str matching-capabilities))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, you probably want to use json/write-str here

@Parsifal-M
Copy link
Author

Nice! Left some comments, but this is a good first attempt.

About the .lsp abd .clj-kondo directories, I think they're added by the VS Code plugin. Those directories only make sense in the core subdirectory, so should probably be added to gitignore here. You can delete them from the PR.

Nice! Will work on these thanks for reviewing so far 👍

…ignore to ignore the kondo and lsp stuff

Signed-off-by: Peter Macdonald <macdonald.peter90@gmail.com>
Signed-off-by: Peter Macdonald <macdonald.peter90@gmail.com>
@Parsifal-M
Copy link
Author

Hey 👋

Still working on this, currently, the output is an empty gencaps.json file, or rather inside it is just "()" (returning nil) so I think something funky is going on with the matching

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants