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

yaml instead of json #148

Merged
merged 12 commits into from
Jun 22, 2022
Merged

yaml instead of json #148

merged 12 commits into from
Jun 22, 2022

Conversation

sideeffffect
Copy link
Contributor

Continuing the conversation from #7 (comment)

There are other interesting candidates for format for configuration besides JSON: JSON5, TOML, HOCON, ... However, YAML is the best choice, because:

  • it is the most widely used
  • has good Scala support
  • is a superset of JSON, allows for smooth transition for Bleep users (if they choose to do so)

@oyvindberg
Copy link
Owner

Interesting, thanks! I'll play around with it tonight and gather some thoughts around this. I'm positive about it, but I won't commit to merging it yet.

If you have time, could you:

  • Remove JSON filename support
  • make same change in bleep.json as in build.sbt
  • run tests so we can inspect what the new files will look like
    ?

@sideeffffect
Copy link
Contributor Author

native-image failing only on Windows 😞

Fatal error: com.oracle.svm.core.util.VMError$HostedError: should not reach here
	at com.oracle.svm.core.util.VMError.shouldNotReachHere(VMError.java:64)
	at com.oracle.svm.hosted.jdk.JNIRegistrationSupport.makeShimDLL(JNIRegistrationSupport.java:273)

This is the difference in logs
image

I guess the only things we can do is report it to GraalVM... the only issues I was able to google are these, but they don't seem relevant at first glance:
oracle/graal#3393
oracle/graal#3659

@sideeffffect
Copy link
Contributor Author

Reported bug for GraalVM native-image oracle/graal#4659

@sideeffffect
Copy link
Contributor Author

@oyvindberg Let's suppose that the Windows native-image issue is sorted for a moment now.
Are you suggesting to drop the support for mere JSON? Isn't that going too far? Having the ability to go with JSON only could appeal to some people, YAML isn't uncontroversial. And since JSON is a subset of YAML, we can have both.

@oyvindberg
Copy link
Owner

Absolutely, if we do yaml we drop json. There are no legacy users to consider, and I only want flexibility in this tool where it is absolutely needed.

@sideeffffect
Copy link
Contributor Author

Cool, fair enough 👍

@oyvindberg
Copy link
Owner

I pushed a commit now with further changes, you can also see yaml versions of the build file for bleep itself, and for the four projects which are used for testing. the test suite does a full import of an sbt build for each project and generates a corresponding bleep.yaml file so we can track changes in behavour over time

@oyvindberg
Copy link
Owner

the build files do look a whole lot better, I'll give it that

@oyvindberg oyvindberg changed the title Add support for YAML: configuration can now also be in bleep.yaml yaml instead of json Jun 20, 2022
@hamnis
Copy link
Collaborator

hamnis commented Jun 21, 2022

  1. fan of choosing one format
  2. dont let that format be yaml, as there are too many known problems with it.
  3. yaml is not really a superset of json
  4. Also this

@oyvindberg
Copy link
Owner

I'm sorry @hamnis, it's going to be json or yaml. Someone else will have to fight the good fight of making the world choose a slightly less terrible way of expressing data.

Prioritized list of things I care about:

  1. ubiquitous tooling support (syntax highlighting, json schema or similar with auto-complete in IDE)
  2. familiarity
  3. readability

slight win for json on 1) and 2), while yaml wins 3) by a very large margin.

Have a look at this build file for instance. I think it looks rather nice!

I strongly dislike yaml as well for all the reasons you gave, but it's better to read and write than json. and no other data language is close in terms of tooling support OOTB and familiarity.

@oyvindberg
Copy link
Owner

that said, this snakeyaml library needs to be replaced somehow. are there any other options out there?

@oyvindberg
Copy link
Owner

scalacOptions do not look their best though, it's rather unfortunate with the dashes.

templates:
  template-common:
    platform:
      name: jvm
    scala:
      options:
      - -encoding
      - utf8
      - -feature
      - -language:experimental.macros
      - -language:higherKinds
      - -language:implicitConversions
      - -unchecked

@oyvindberg
Copy link
Owner

maybe as a multi-line string instead, I've heard those are awesome in yaml :D

@oyvindberg
Copy link
Owner

turns out there is a successor library called snakeyaml-engine, which only does yaml 2.4 (which seems like it avoids some of the common problems with yaml). it seems to be of much higher quality, so I don't suppose it will cause the same issue. crucially, it has no support for the old, bastardized java beans pattern at all.

I forked circe-yaml, updated it to that new library. necessary changes https://github.com/circe/circe-yaml/compare/master...bleep-build:snakeyaml-engine?expand=1 . There are tests for some old yaml-crap which need to be updated before it can be upstreamed.

@sideeffffect
Copy link
Contributor Author

sideeffffect commented Jun 21, 2022

snakeyaml-engine does YAML 1.2
see https://bitbucket.org/snakeyaml/snakeyaml-engine/src/master/README.md

@oyvindberg
Copy link
Owner

haha cool, I read that as 2.4 and kept writing that everywhere

@sideeffffect
Copy link
Contributor Author

circe/circe-yaml#303 🤞

@sideeffffect
Copy link
Contributor Author

      options:
      - -encoding
      - utf8
      - -feature
      - -language:experimental.macros
      - -language:higherKinds
      - -language:implicitConversions
      - -unchecked

can be just

options: ['-encoding', 'utf8', '-feature', '-language:experimental.macros', '-language:higherKinds', '-language:implicitConversions', '-unchecked']

Ondřej Pelech and others added 8 commits June 22, 2022 09:35
concerns the following files:
$PROJECT/bleep.yaml (build file)
$PROJECT/.bleep/bsp/project-selection.yaml (used to only mount some subset of projects in IDE)
$HOME/.config/bleep/config.yaml (user-level config)
@oyvindberg
Copy link
Owner

ok, we'll iterate on this going forward, but I'm feeling good enough about it now. In it goes, thanks @sideeffffect

@sideeffffect sideeffffect deleted the yaml branch June 22, 2022 08:50
@sideeffffect
Copy link
Contributor Author

Nice 🚀 thank you too

oyvindberg added a commit that referenced this pull request Jun 23, 2022
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.

None yet

3 participants