Fork me on GitHub
#calva
<
2020-03-05
>
sogaiu01:03:30

@pez i was looking at the tm grammar and started wondering why nil is handled differently from true and false: https://github.com/BetterThanTomorrow/calva/blob/master/clojure.tmLanguage.json#L71 https://github.com/BetterThanTomorrow/calva/blob/master/clojure.tmLanguage.json#L75 perhaps an accident of history? or may be some other reason? it appears that if you put a comma after a nil, (<- like that) highlighting goes away, though that doesn't happen for true and false (which i guess is preferred?)

sogaiu01:03:30

i think i'm using 2.0.79, fwiw

pez05:03:38

Good observation! Most of that grammar is inherited from Atom’s clojure language. The issue with comma after nil should be fixed.

sogaiu05:03:13

would removing the lookahead assertion for the nil regex be sufficient?

pez09:03:02

It's all pretty well coverd by tests, so I it is reasonably easy and safe to experiment with it.

pez09:03:05

W/o having looked at it, from your description, it sounds like we should treat it much more like we treat booleans.

sogaiu10:03:02

when you say tests, do you mean something different from: https://github.com/BetterThanTomorrow/calva/wiki/Smoke-Testing ?

bringe00:03:58

I believe he means code tests

bringe00:03:04

Sorry, had not scrolled my convo yet and didn't see newer messages 😂

sogaiu00:03:58

np -- appreciate all the help i can get!

simple_smile 4
pez10:03:14

TLDR: If you are hacking on that, please holler at @pez on the Clojurians slack, and I will give you some starting points.

sogaiu10:03:38

i suppose my voice doesn't have to be raised -- as it already seems to be working 😉

pez10:03:40

So, I test them using Atom. Run Package Specs

pez10:03:07

And the CI pipeline also does that.

pez10:03:58

It is quite simple, and I should describe it on that wiki page.

sogaiu10:03:00

i think i may be missing something as i get an error:

index.js:129 Error: Cannot find module '/home/user/src/calva/spec'

sogaiu10:03:10

that shows up in a new atom window in the devtools portion

sogaiu10:03:27

i don't see any spec file or directory in my calva source directory

pez10:03:14

1. Open the src/calva-fmt/atom-language-clojure directory in Atom. 2. Run Package Specs 3. Update the grammar (I do that in VS Code, because I love VS Code): https://github.com/BetterThanTomorrow/calva/blob/master/src/calva-fmt/atom-language-clojure/grammars/clojure.cson 4. In Atom Run Package Specs You probably need to update the specs too, depending on the change: https://github.com/BetterThanTomorrow/calva/blob/dev/src/calva-fmt/atom-language-clojure/spec/clojure-spec.coffee

sogaiu10:03:48

ah great -- that works

pez10:03:17

Awesome. I updated the wiki now. 😃

sogaiu11:03:41

sounds good 🙂 ok, changed the .cson grammar and then tests passed. just to make sure i also changed it to something incorrect and verified the tests would fail then. the clojure-spec.coffee file appears to mention nil only here: https://github.com/BetterThanTomorrow/calva/blob/dev/src/calva-fmt/atom-language-clojure/spec/clojure-spec.coffee#L88_L90 may be this is a case of not having to change the specs file?

pez11:03:50

I'd appreciate a change that would expose the error you fixed. 😎

sogaiu11:03:20

i'll see what i can do -- i'm neither a coffeescript writer, nor a spec writer, but am willing to learn

pez11:03:58

It’s my only exposure to those as well. It’s pretty nice.

sogaiu11:03:13

ah, what do you like about them?

sogaiu11:03:25

btw, the text here doesn't quite look right:

Made sure I am changed the default PR base branch, so that it is not `master`.

pez11:03:53

Grammatically ?

sogaiu11:03:57

"I am changed the default" doesn't make sense to me. wdyt?

pez11:03:15

I agree. Just wasn't sure it was what didn't look right. I've struggled quite a bit to not have to have that text at all.

sogaiu11:03:17

ah -- i think having reminders isn't a bad idea, fwiw 🙂

sogaiu11:03:35

i think i can probably build a vsix and test it here. puzzled a bit by:

You'll find the artifacts by clicking _Show all checks_ in the CI section of the PR page, and then _Details_ on the `ci/circleci: build` test.
without submitting the pr, i presume i don't seen any of that. does that sound right?

pez11:03:01

Yeah. It happens as a result of the push.

sogaiu11:03:29

so the idea here is to submit the pr, but the checklist is not supposed to be completed until after i guess?

pez11:03:43

Please feel invited to update that wiki.

sogaiu11:03:02

err, i'm just reading the template in the pr text field 🙂

pez11:03:13

Yeah, I have probably assumed something when writing those instructions.

pez11:03:05

Right, it is part or the PR template... You are welcome to edit that too of course. But I have made a note about both errors, so will fix'em.

sogaiu11:03:26

sounds good -- just trying to follow instructions here 😉

pez11:03:25

Appreciated. I'm a fan of using checklist to try stop problems I have encountered.

sogaiu11:03:39

sure me too -- too many things to remember!

pez11:03:22

It was a very long time ago a working VSIX built locally didn't mean it also worked from CI. But since that is what is going to be what is released, I think I want to keep that check step.

pez11:03:52

However. CircleCI makes it really hard to find that artifact. I should update the instructions for that as well. That, or move to Github Actions.

sogaiu11:03:41

yes, it makes sense to actually test what is going to be released. if we had reproducible builds, i'd much prefer to install what i built locally though.

borkdude12:03:15

@pez FWIW, for babashka I made a script which posts the artifact links to #babashka_circleci_builds

borkdude12:03:26

CircleCI artifact links that is

pez12:03:08

Very cool! But the fact that this is needed serves to convince me it is time to leave CircleCI...

sogaiu12:03:11

@pez i've looked for an example of: "Referenced the issue I am fixing/addressing in a commit message for the pull request." -- didn't find one in a brief search. do you have an example i could emulate?

pez12:03:53

If I understood the question correctly?

sogaiu12:03:31

ah, i see -- ty. so you put the fixes and issue number in the body, not the first line.

pez12:03:18

Yeah. It creates a nice trail on GH (but you probably have figured that that is why this is done).

pez12:03:40

I think that today it might work to only mention this in the PR text, but I am not sure.

sogaiu12:03:03

i'm trying to do the new ui experience thing -- confused by selecting the organization step. does that matter?

pez12:03:43

It is totally confusing... What I do these days is that I edit the URL for one of the other tabs in the old UI.

sogaiu12:03:34

somehow i managed to find the tab 🙂

pez12:03:53

If you know the secret, please share. 😃

sogaiu12:03:29

lol -- well, after choosing the new ui experience and going through the oauth2(?) dance, i went back to the pr and followed links from there.

sogaiu12:03:32

that seemed to do it.

pez12:03:30

OK. so basically my hints were enough? 😃

sogaiu12:03:52

i guess i would say that the moral support was helpful 😉

sogaiu12:03:00

the vsix worked in a vm, fwiw

metal 4
borkdude12:03:19

@sogaiu Are you making a PR for treesitter into Calva?

borkdude12:03:52

(no really, I thought it was something related to that maybe)

sogaiu12:03:55

i have made some progress on the tree-sitter clojure grammar. but iiuc, getting the grammar to work with themes in vscode is not really going to work atm.

sogaiu12:03:21

the pr is vaguely related in that i'm studying highlighting and found a minor issue in the current tm grammar.

sogaiu12:03:38

@borkdude thanks for asking though 🙂

borkdude12:03:47

@pez Posted a reply in that thread. Also informed @marc-omorain

👏 8
sogaiu12:03:23

@pez ok, have made my pr attempt. please let me know how i did at some point 🙂

pez12:03:04

Process-wise you totally win every price, so far. ❤️

sogaiu12:03:54

let's hope the quality of the actual work is good 😉

pez12:03:12

Totally agree. I will check that tonight and let you know. 😃

pez12:03:54

What I meant by process-wise prices was that you ask when you wonder something. Sometimes I suspect people just throw up their hands and never lets me know when I have been unclear.

sogaiu13:03:47

well, i tend to be at the other extreme -- or may be you noticed already 😅

😍 4