Clojurians
#parinfer
<
2016-03-14
>

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

cfleming04:03:34

I have JVM Parinfer updated to 1.7.0, I’ll probably push it tomorrow.

cfleming04:03:24

I’m going to start numbering the versions to match Parinfer proper, too.

chrisoakman04:03:24

I suppose that makes sense so long as it tracks parinfer implementation pretty closely?

cfleming04:03:42

At the moment they’re basically identical.

cfleming04:03:16

I mean, I can version it independently, but I’m not sure how much sense that makes. Currently the idea is that the JVM version is basically an exact port of the original, right?

chrisoakman04:03:06

What happens when you make a change that doesn't match parinfer.js? Like a JVM speed improvement.

chrisoakman04:03:00

The idea of semver is that the public API regulates your version numbers

chrisoakman04:03:28

I think the public API and major version should probably track parinfer.js closely, but I can see situations where parinfer-jvm would have minor or patch releases that differ from parinfer.js

chrisoakman04:03:30

parinfer.py 0.7.0 is a good example of a release that only changed internals for Python-reasons: https://github.com/oakmac/parinfer.py/commit/eff391b444e6944737107d027d0abd38b2aa27a6

chrisoakman04:03:10

Want to just flag parinfer-jvm at 1.0.0 with your next release? I'm fine with that; I don't think the public API is going to change in a breaking way anytime soon

chrisoakman04:03:20

You are probably the only consumer of parinfer-jvm anyway :wink:

sekao05:03:49

@chrisoakman: actually i’m using it too :smiley:

cfleming06:03:35

@chrisoakman: Yeah, I was thinking about that on the way home and came to the same conclusion. I’m actually going to look at changing the API to make it throw an exception on error rather than catching it and wrapping it, which is fairly idiomatic and makes the result object simpler.

cfleming06:03:44

@sekao: Any complaints if I do that?

chrisoakman06:03:57

hmm; I also need to give that a think

chrisoakman06:03:11

that would be different from all the other ports of Parinfer

chrisoakman06:03:27

the exception catching mechanism inside Parinfer isn't really an error in the classic sense of "something went wrong and I don't know how to deal with it"

chrisoakman06:03:00

it's perfectly valid (and expected) for the user to type text in Indent Mode that Parinfer simply cannot process (unbalanced quotes are the best example of this)

chrisoakman06:03:37

the result object returns success: false in those cases

chrisoakman06:03:07

but it's nice that the result from Parinfer is always in the same format

chrisoakman06:03:30

I don't really know what it would mean if Parinfer threw an error in those cases; whatever would be trapping that error would have to know about Parinfer internals to be able to understand or deal with it

chrisoakman06:03:03

and, if the user of the API doesn't wrap the Parinfer try in a catch, then something pretty innocent (like passing an unbalanced quote string) could theoretically blow up very high up the in stack

chrisoakman06:03:42

I think maybe Parinfer exceptions should be constrained to the Parinfer library

chrisoakman06:03:25

parinfer.js didn't use exceptions in the past; it just kept some additional boolean flags on the result object and checked them at every loop iteration

chrisoakman06:03:04

(not that that is better than the way it currently works, just an interesting observation)

cfleming08:03:01

It’s more idiomatic for Java libraries, though. And (checked) exceptions in Java are usually used for conditions that are expected (file not found), and runtime ones for unexpected (null pointer etc)

cfleming08:03:58

The exception isn’t an unknown part of the Parinfer internals, it becomes a part of the API at that point. It’s unusual to return an error object in Java libs.

cfleming08:03:33

> if the user of the API doesn't wrap the Parinfer try in a catch, then something pretty innocent (like passing an unbalanced quote string) could theoretically blow up very high up the in stack You’re inadvertently making the case for checked exceptions :simple_smile:. And the same is true if you return a result object where some fields are filled in on error and others on success - if you don’t correctly check the error cases you’ll get a null pointer with the same effect.

cfleming08:03:39

An error like that should be a one-time event during development, too, unless the implementer never checks any error conditions either in manual testing or test cases. That’s unlikely here since there’s an established test suite.

cfleming09:03:27

> it's nice that the result from Parinfer is always in the same format Except that it’s not, really. There are fields that are filled in in case of error, and fields that are filled in in case of success. If you think in terms of schemas they’re really two distinct cases. Using an exception with the error fields, and a result object with the success fields is IMO cleaner and means that the fields don’t have to be nullable.

cfleming09:03:18

I don’t think it’s a big deal though, it’s just a little odd for Java consumers right now.

sekao12:03:08

interesting discussion. i gotta admit, i’m indifferent :wink: i guess it depends on whether your goal is to be a direct port of the reference impl or be an idiomatic library in its own right. i thought the goal of parinfer-jvm was primarily the former. if it was the latter, i imagine you’d want to change other things as well, like (heaven forbid) use a stateful object with instance methods instead of static methods.

cfleming21:03:27

Yeah, although I think static methods in APIs are more common than error objects. Either way, I agree that the worldwide anticipated audience for this port probably approximates 2, so I’ll leave it as is right now.

cfleming23:03:20

I just pushed 0.4.0 of parinfer-jvm, tracking parinfer proper 1.7.0.

cfleming23:03:28

This has the previewCursorScope fix.

cfleming23:03:12

I’ll stick with the current numbering for the moment - @sekao was having a strange problem that I haven’t had time to investigate, I’d like to fix that before pushing a 1.0.