Fork me on GitHub
#parinfer
<
2017-06-09
>
shaunlebron00:06:43

not crazy about the name

shaunlebron00:06:15

wanted it to be slightly related to parinfer, without creating an impression that it forces everyone to use parinfer if it’s used

dominicm08:06:54

@shaunlebron fraction of a second on 40k loc

dominicm08:06:54

2.5s on 31k loc

dominicm08:06:13

0.77s on the 40k loc.

dominicm08:06:58

@shaunlebron changes lgtm. Biggest change on one project was that a bunch of #_-related indents.

dominicm09:06:55

Not sure if this is intentional, but:

(let [x 1
      ]
  x)
Seems to leave behind an empty line where the ] was

dominicm09:06:34

also multi-line strings don't get indented. E.g.:

(defn foo
"Hello
world")
(defn foo
 "Hello
world")

shaunlebron14:06:05

@dominicm thanks for feedback!

shaunlebron14:06:54

i’ll try adding an option to delete lines that were left empty after linting

shaunlebron14:06:01

multi-line strings cannot be indented, since that would be adding more literal spaces inside the string, thereby breaking code!

shaunlebron14:06:29

i’ll add a note about that 👍

dominicm14:06:00

@shaunlebron yeah, I thought that might be the case. There's perhaps an "edge case detection" where you say "This is all aligned already, and it's inside a defn, let's indent it & warn the user". But I'm not sold on that being a good approach.

shaunlebron14:06:22

ah, I see what you mean

shaunlebron14:06:28

for things like docstrings I suppose

shaunlebron14:06:50

where it doesn’t really matter

shaunlebron14:06:02

docstrings are a bit inconsistent in the cljs compiler code—sometimes it’s indented to the first ", other times is it’s indented one space to the right of it

dominicm14:06:10

I know. 😞

shaunlebron14:06:14

and sometimes it changes within the same docstring

shaunlebron14:06:30

very hard to detect

dominicm15:06:22

I think Clojure's multi-line string support isn't necessarily great tbh. I wish I could do:

<<EOS
If you provide "foo" as a parameter…
EOS
for example.

dominicm15:06:50

I agree, I notice it a lot. I imagine zprint is the tool for fixing that (I didn't check!)

dominicm15:06:01

it doesn't 😞

shaunlebron15:06:03

would you throw a reader error if text inside the heredoc was indented to the left of the << ?

shaunlebron15:06:20

i’ve heard these complaints before, but I’m not sure how people would solve it

dominicm15:06:05

@shaunlebron I'm sure there's issues 🙂. I'm not really sure what a heredoc is! I just know it would be nice not to have to worry about string escaping at times, for doing expressive docs.

shaunlebron15:06:05

@dominicm heredoc is the common name for the <<EOS syntax for describing multiline strings

shaunlebron15:06:36

anyway, definitely don’t commit those linted files yet, the trim will have to know about the original code so it doesn’t remove the other empty lines

dominicm15:06:37

@shaunlebron ah! I see. I guess the current bash/ruby implementations would be worth comparing against. I don't really feel strongly, I'm just currently thinking about docs. No doubt I'll end up at literate programming.

dominicm15:06:51

Oh, I already landed that bomb 😉

dominicm15:06:13

I fixed it manually, vim made it easy enough, especially as I checked every change manually so I'd have feedback.

shaunlebron15:06:18

ah, the perils of being a beta tester

shaunlebron15:06:38

cool 👍 appreciate it. good feedback

shaunlebron15:06:25

@dominicm did you have to negotiate at all with your team?

shaunlebron15:06:41

and did you integrate it as a check?

dominicm15:06:25

@shaunlebron I ran it as a one-off for now. No negotiating required. The changes were mostly sane. I also manually reverted a change around https://github.com/shaunlebron/parinfer/issues/92

dominicm15:06:10

It was easy to use parlinter on the codebase because it's not invasive at all. Nobody is going to fight fixing confusing indentation & moving parens onto the same line.

shaunlebron15:06:14

I’m documenting the cases where people might fight it

shaunlebron15:06:47

you’re saying you reverted the comment change to keep the paren after the comment?

dominicm15:06:21

I also think using git ls-files -- **/*.{edn,clj{,c,s}} for the list of inputs to parlinter would have been faster, as parlinter probably ran against the compiled files unnecessarily.

dominicm15:06:42

uh, you suggest **/*.{edn,clj,cljc,cljs} but same thing

shaunlebron15:06:42

do you think the comment case can be resolved in Indent Mode somehow? that’s a scary thing to me but I’ll try to revisit

dominicm15:06:54

@shaunlebron I've no idea. I know you didn't like the edge case of it. It's definitely kind of incompatible with parinfer. But it's definitely the sort of thing I see in codebases quite commonly, and removing it isn't really okay where we have it (usually detailing the reasons for elements in a vector, or documenting why some function unexpectedly appears at the end of the threading macro)

dominicm15:06:44

> quite commonly Rather, I see it often in codebases "at least once". So, I don't see it everywhere. But I usually do see it. Not sure how to word that.

shaunlebron15:06:58

I suppose (comment) would work better, but would be inconsistent with the other ;; comments

shaunlebron15:06:58

my concern is that it won’t be integrated as a checker if it fails on an edge case that people want to keep around, and I’ll have to add a ;; parinfer: ignore directive

dominicm15:06:04

Well:

(-> 10
    distinct (comment This is like this because reasons of #1 XYZ, #2  DEH))
A. syntax highlights B. Triggers the tag reader

shaunlebron15:06:31

maybe as a string

shaunlebron15:06:46

yeah, it’s just bad

shaunlebron15:06:26

(-> 10
    distinct) ;; This is like this because reasons of #1 XYZ, #2  DEH

shaunlebron15:06:55

obviously that’s not an illustrative case for the problem we’re describing though

dominicm15:06:23

I think the big benefit you get with

(-> 10
    distinct ;; This is like this because reasons of #1 XYZ, #2  DEH
    )
Is that it's clear you're targetting the distinct vs the the whole (->)

dominicm15:06:57

The trailing paren hurts me though. I agree that it's an annoying edge case.

shaunlebron15:06:01

(-> 10
    distinct
    ;; This is like this because reasons of #1 XYZ, #2  DEH
    (comment))

(-> 10
    distinct
    ;; This is like this because reasons of #1 XYZ, #2  DEH
    #_ignore_me!)

shaunlebron15:06:34

it can be used as a sentinel maybe

dominicm15:06:39

The answer is probably to do this:

(-> 10
     ;; This is like this because reasons of #1 XYZ, #2  DEH:
    distinct)

shaunlebron15:06:04

that works, but (+ 1 2 (comment)) is 3, and I think that’s the only way to create a sentinel to allow Parinfer to keep the paren after a comment

shaunlebron15:06:11

oh, and #__ would work too

dominicm15:06:41

Not entirely great to be writing code specifically for parinfer though. My team don't use parinfer, so would probably ? over such code

shaunlebron15:06:33

yeah I agree, was just tempted by the technical novelty of the solution i hadn’t realized before

dominicm17:06:03

I have another codebase that can have the linter treatment btw, so let me know when trimming all works

shaunlebron19:06:37

@dominicm trimming seems to work if you wanna give it a try

dominicm20:06:17

just for fun, usage from vim: !aFparlinter --trim --stdin

dominicm20:06:21

A case where using ;; Foo is like cause X: doesn't work:

{:foo 1
 ;;:bar 10
}

dominicm20:06:49

again, uncertain how I feel about it 🙂 totally understand why it is how it is.

dominicm22:06:20

@shaunlebron https://github.com/shaunlebron/parinfer/tree/master/lib#api > full text input The whole file? Does it have to be? Or can it be the top level form > cursorLine Relative to the section of text, or whole file?

shaunlebron22:06:07

atom uses it to format top-level forms

shaunlebron22:06:23

where the cursor is relative to the form

dominicm22:06:43

Okay, cool. I think the nvim parinfer slow down may be due to it running on whole file. Currently hell on a 1.3k line file. Gonna try optimise the plugin 🙂

shaunlebron22:06:41

@dominicm strange, not sure what the slowdown could be

shaunlebron22:06:37

also, both zprint and cljfmt allow parens after a comment 😕