Fork me on GitHub
#calva
<
2022-02-11
>
Cora (she/her)03:02:33

I'm trying to improve the eslint settings for calva and big oof on all these errors

Cora (she/her)03:02:03

✖ 2757 problems (2439 errors, 318 warnings)
  445 errors and 2 warnings potentially fixable with the `--fix` option.

Cora (she/her)03:02:34

I probably need to relax a lot of rules and fix things gradually to have tighter types

pez06:02:33

We’ve not been using eslint much. More worked from the tsconfig. And also generally been lax and opt-in with the types.

pez06:02:57

We’ll be happy to consider your advice around how we can tighten it up. We’ll just need to understand the trade-offs.

pez06:02:47

We prefer that changes like these come in separate PRs from bug fixes and feature adds/changes. It gets tricky to reason about two things at the same time.

Cora (she/her)06:02:49

I'm happy to pitch in

Cora (she/her)06:02:27

and gradual changes with PRs w/ discussion (and separate from feature work) was what I was hoping to do

1
pez07:02:19

How does eslint relate to tsconfig?

Cora (she/her)07:02:24

tsconfig.json is the configuration file for typescript. .eslintrc.js/`.eslintrc.json` are configuration files for the linter, eslint. eslint has plugins that add rules, rules that prohibit or require specific ways of writing bits of code. For example, here is an eslint plugin that enforces rules around using promises https://github.com/xjamundx/eslint-plugin-promise#rules

Cora (she/her)07:02:06

there's an eslint plugin for linting typescript code as well: https://typescript-eslint.io/rules/

Cora (she/her)07:02:45

so for instance you can ban the use of specific types if you'd like https://typescript-eslint.io/rules/ban-types

Cora (she/her)07:02:24

there's a plugin that makes eslint work better with prettier https://github.com/prettier/eslint-config-prettier

Cora (she/her)07:02:42

does that make sense?

pez07:02:48

It will probably once I've pondered it a bit. 😃 Thanks for the information!

💜 1
pez07:02:25

Let's take the example you give with the ”rules around promises”. Iirc we have some rules for the in tsconfig. (I should check, but I'm in a hurry here). Does this mean that while eslint and tsconfig have different rules, there's also some overlap?

pez07:02:27

I'm asking because every time VS Code has kindly offered me ESLint, I have replied, ”thanks, but I'm cool, I have tsconfig” 😃

pez07:02:47

Some of those promise rules make a lot of sense, btw.

Cora (she/her)07:02:15

There isn't really anything in the tsconfig.json about promises per se, but typescript itself requires you to type async functions correctly (so like Promise<MyReturntype). eslint just goes a step further with ensuring code style stuff

Cora (she/her)07:02:18

so I don't think it's in effect

pez07:02:27

Ah, yeah, maybe that was what I was remembering seeing...

pez07:02:27

No, I am thinking about tslint.json. This is the level where I am confused about this.... So, I guess my real question is how tslint and eslint relate. 😃

Cora (she/her)07:02:51

ahhhh yes, tslint. tslint was replaced by eslint

Cora (she/her)07:02:07

> ⚠️ TSLint https://medium.com/palantir/tslint-in-2019-1a144c2317a9 as of 2019. Please see this issue for more details: https://github.com/palantir/tslint/issues/4534. https://typescript-eslint.io/ is now your best option for linting TypeScript.

Cora (she/her)07:02:09

that was a painful move, from tslint to eslint, but eslint feels a ton better now that it supports all the things

Cora (she/her)07:02:55

it's extremely late for me, I need to head to bed

pez07:02:00

Shouldn't be too painful with out tslint with it's sole rule. 😃

Cora (she/her)07:02:29

I'll hop on tomorrow night and maybe fix that wiki entry (assuming the PR got merged)

Cora (she/her)07:02:42

take care! 💜

💜 1
Cora (she/her)03:02:41

assuming this is desirable to the maintainers

pez07:02:47

Dear Calva friends. We're closing in on 1K members in this channel! 🎉 I feel like celebrating, but don't know how to. 😃

🍻 7
🎉 7
catjam 3
pez07:02:21

Is this the second time we approach 1K, btw? Or just a glitch in the matrix for me?

bringe16:02:28

You might be thinking of 1k GH stars.

pez16:02:35

Yes. We celebrated that a bit, so probably what I am remembering. 😃

borkdude20:02:18

Congrats!

🙏 1
metal 1
calva 3
pez10:02:17

1K members now! 🎉 calva

🥳 7
👍 2
catjam 3
pez07:02:03

Attention, Calva contributors, especially you who have pending PRs: We've just merged a 10kloc change unifying the formatting across the whole TypeScript part of the project. Thanks @corasaurus-hex! You'll need to merge in dev. If you get in conflict troubles, maybe it is easiest to first apply the Prettier changes to your branch and then merge (purely speculating here). Anyway, I'm sorry for any inconvenience, but I am very happy that I won't have to keep asking people to clean their PRs from (often proper) formatting changes. https://github.com/BetterThanTomorrow/calva/pull/1524

pez08:02:32

@U9A1RLFNV should we maybe do a release with this? Just to make sure that published gets with the program asap and won't give us trouble when we want to release fixes/changes.

bringe16:02:57

That’s probably a good idea. I probably wouldn’t get to that today, myself, though.

pez16:02:07

I'll do it now then. since you agree. 😃

pez16:02:18

Ah, good that I did this. https://app.circleci.com/pipelines/github/BetterThanTomorrow/calva/4357/workflows/67a9329c-3958-41e6-ac20-6bf239f29191/jobs/17631 This works in the other steps so must be something special with the cibuilds/github:0.10 image. What's the -T option on cp supposed to do, @corasaurus-hex? I don't have it on MacOS either. 😃

Cora (she/her)17:02:17

i can check on this in an hour or two, sorry for the trouble. odd that it was working before but now now

Cora (she/her)17:02:32

it's an option for gnu's cp though

pez17:02:09

The github release step is only run when we publish a new release, so this has probably not been working before.

pez17:02:03

There is no hurry what so ever. I just wanted to make sure the pipeline is still fine.

pez17:02:22

This put a smile on my face

💜 1
Cora (she/her)17:02:58

it can be replaced with rmdir ~/calva && cp -r /tmp/build ~/calva

Cora (she/her)17:02:44

the -T just prevents copying the build dir into /calva if /calva already exists

pez17:02:14

-T is a power move!

Cora (she/her)17:02:45

and it's replacing cp -r /tmp/build/* ~/calva because that command doesn't copy hidden files (like the .prettierrc.js)

Cora (she/her)17:02:18

on macos you can install gnu coreutils via homebrew and then use gcp instead of cp

pez17:02:11

I have coreutils installed, actually. Don't recall why, but probably since when I created with the Calva CI pipeline and needed to run the jobs locally.

Cora (she/her)18:02:26

yay!! thanks, Peter!!

borkdude19:02:52

@alexmiller Answered.

😎 1
❤️ 1
borkdude 2
Isac21:02:26

Hi folks, How I can use the https://calva.io/formatting/ using the clojure-lsp formatting instead of cljfmt?

ericdallo21:02:53

@U9A1RLFNV @U0ETXRFEW this is not the first time it's being asked, should we add a flag to use clojure-lsp formatting instead of calva's one disabled by default for now?

bringe21:02:03

I support that. What do you think, @U0ETXRFEW?

pez21:02:36

We probably should. But it is not clear cut. Calva formats as a result of a lot of operations like explicitly Format Current Form and after structural editing. So a user who selects clojure-lsp as the formatter with such an option will still be using Calva's formatter a lot. This risks being confusing. That said, I don't actually know what the difference is for the user when using clojure-lsp instead of Calva's formatter.

ericdallo21:02:26

because with LSP it's possible to centralize a config for multiple projects and access it via classpath (check https://clojure-lsp.io/settings/#classpath-config-paths), but this is not possible with only calva formatter

ericdallo21:02:31

we use this at nubank a lot for example

ericdallo21:02:29

also, since Calva relies on clojure-lsp for a lot of things, in a long term, having all formatting features in a single place sounds like a good idea to me

ericdallo21:02:54

I think we should at least offer a option for user, to when they use the format command manually or via shortcut uses clojure-lsp format

pez22:02:22

Thanks. Still it risks confusing people to have different formatting happening in different situations, that may not seem so different to them.

ericdallo22:02:51

But it's behind a flag, at least a option to offer to users, or can´t we have a flag to make all places calva formats uses that at least?

pez22:02:04

Yes, as I said, we probably should have this flag. I’m merely anticipating the confusion that will ensue. :face_with_cowboy_hat:

👍 1
bringe22:02:36

Does clojure-lsp offer formatting just a specific form?

ericdallo22:02:25

it supports both formatting and rangeFormatting, the latter you can pass a line/col range

👍 1
pez22:02:22

It could be worth exploring how Calva could use the configuration from clojure-lsp instead. I’m pretty sure most users don’t care which system is doing the formatting.

ericdallo22:02:52

yeah, the cljfmt information is already available via serverInfoRaw request, so I agree that could work as well

ericdallo22:02:36

it sounds like a good idea as a first step, and maybe in the future we could just rely on clojure-lsp after we find what would be enough to change to support what calva has ATM

bringe22:02:38

If we want to eventually stop using Calva’s fork of cljfmt anyway, it might even make sense to eventually rely entirely to clojure-lsp for formatting.

👍 2
Lukas Domagala22:02:11

It kind of depends if we still want to be able to run without lsp. I like pushing features down the stack, but we probably should have some basic support for running without lsp

bringe22:02:59

Yes, that’s the main question with decisions like these.

ericdallo22:02:32

I thought calva was already using a lot of LSP features, I don't see we removing that support right? If we disable LSP via a flag it's intended to disable it's related features I think

bringe22:02:28

I’d like to be able to ease maintenance and free up time for other features, deferring to clojure-lsp for the things it offers, and remove the related code from Calva completely. I feel that can benefit Calva and Calva users far more than it might be a downside, overall. But that’s just my viewpoint, and might not be shared by others.

☝️ 2
Lukas Domagala23:02:43

Yeah we use most of lsp, but can also use nrepl for most of it or have stuff like fmt internally. I'm not arguing against using it, just not sure we should remove a working feature. It's more of a calva vision question than a technical one. But @U9A1RLFNV is also right that the time could be spent on other things

ericdallo23:02:25

Yeah, I agree, that's why my suggestion is just a flag for now :p Then we can see how calva users think about it and help decide the future as well

👍 1
pez23:02:35

That escalated quickly :face_with_cowboy_hat:

😈 3
orestis16:02:33

The more standard tools are there the better to interop with other people too. If Clojure-lsp can format, that's also accessible from emacs, command line etc.

pez16:02:46

Not sure I follow, @U7PBP4UVA?

pez16:02:58

In any case, Calva's formatter isn't going to be dropped. It would make clojure-lsp a requirement for formatting, and we are rather going to try make clojure-lsp a super-nice, default enabled, add-on.

Isac13:02:16

I'm using the following versions below and my format on save doesn't work. I install the latest clojure-lsp from #clojure-lsp-builds versions:

calva v2.0.244
clojure-lsp 2022.02.01-20.02.32
clj-kondo 2022.02.09
config:
"calva.fmt.configPath": "CLOJURE-LSP",
"calva.clojureLspPath": "/usr/local/bin/clojure-lsp",
"editor.formatOnSave": true,
error:
Fetching formatting settings from clojure-lsp failed. Check that you are running a version of clojure-lsp that provides "cljfmt-raw" in serverInfo.

ericdallo13:02:51

@U9N15525C just to make sure, you did downloaded latest version from #clojure-lsp-builds right?

ericdallo13:02:02

nvm just saw it

ericdallo13:02:46

@U9N15525C could you try calling calva command clojure-lsp server info and check if there is a cljfmt-raw field?

Isac13:02:54

"cljfmt-raw": "{:indents {as...

pez13:02:01

Strange. Can you file an issue about this on Calva, @U9N15525C?

👍 1
pez13:02:49

Thanks. And thanks for including the full cljfmt-raw info. I was about to ask you to do that. 😃

Ivan Koz22:02:21

Calva doesn't follow to JDK sources on ctrl+click, gives a link to currently open clj file. Any ideas?

Ivan Koz00:02:28

From an lsp log on > goto def command while a cursor is on Math/abs

[Trace - 04:17:35] Sending request 'textDocument/definition - (146)'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/Users/localhost/Projects/test-clj/src/me/nxtk/test_clj.clj"
    },
    "position": {
        "line": 14,
        "character": 8
    }
}


[Trace - 04:17:35] Received response 'textDocument/definition - (146)' in 3ms.
No result returned.

bringe04:02:34

Yes, this currently doesn’t work very well, if at all (maybe it does in certain situations). You can subscribe to that issue for updates.

bringe04:02:57

Clojure-lsp does not provide this support, either.

bringe04:02:35

When Calva does end up supporting this, I believe it may come only from the repl you connect Calva to (or the one you start with jack-in), and will likely not come from clojure-lsp. @UKFSJSM38 Is there a plan for clojure-lsp to provide Java source definitions in the future? I forgot what you’ve said about this before. 😄

Ivan Koz09:02:46

that would be really nice, as a newbie I tend to read source code a lot to figure out what exactly is going on and honestly left click is the fastest way to navigate down the stack

ericdallo13:02:11

There is actually plans to add support for that @U9A1RLFNV , there is this related kondo issue https://github.com/clj-kondo/clj-kondo/issues/1330, @U04V15CAJ and I spent some time thinking on how to achieve that but didn't implement nothing yet

👍 2