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

Allow integration with atom-ide-purescript #63

Closed
Dretch opened this issue Jan 1, 2019 · 20 comments
Closed

Allow integration with atom-ide-purescript #63

Dretch opened this issue Jan 1, 2019 · 20 comments

Comments

@Dretch
Copy link
Collaborator

Dretch commented Jan 1, 2019

I use Atom to write Purescript source code.

When I converted my project from Pulp + Bower to Spago the nice features of Atom (show-type-on-hover, go-to-definition, auto-import, etc) stopped working.

It would be nice to get this working again. I am not sure entirely what is involved, or whether changes are required to https://github.com/nwolverson/atom-ide-purescript itself. One thing that does appear to be required is for spago build to pass through arguments to purs, like pulp build -- --json-errors (once this is done then atom-ide can at least be configured to build with Spago).

@justinwoo
Copy link
Contributor

i think what you're missing is that the language server that's running doesn't know where to find sources. you probably want to open an issue in atom-ide-purescript about providing a way to call a command to get psc-package style sources, so that you can use spago sources to get the source globs.

@f-f
Copy link
Member

f-f commented Jan 1, 2019

From a cursory look at the atom-ide-purescript codebase, it looks like Justin is on point about the plugin not finding the right source globs. Though I'm not sure we have to use spago sources for this: it looks like the default value for the source globs points to the bower cache (here), but this is configurable by setting ide-purescript.pscSourceGlob (see here).

So I guess setting it to something like ["src/**/*.purs", ".spago/**/*.purs"] might work?

@mebubo
Copy link

mebubo commented Jan 1, 2019

FWW, when I tried purs repl src/**/*purs .spago/**/*.purs in a project, the repl failed to load because .spago/**/*.purs matches purs files outside of packages' /src (e.g. .spago/free/v5.1.0/benchmark/Benchmark/Main.purs), and those may fail to compile.

spago sources avoids this by only including /src.

@justinwoo
Copy link
Contributor

For the most part, you should be able to use .spago/*/*/src/**/*.purs for the sources of your dependencies. If you run into problems with conflicting module definitions with this, you could clean your .spago directory and try again.

@f-f
Copy link
Member

f-f commented Jan 6, 2019

Update: once #66 goes in it would be possible to run spago build -- --json-errors, so the only thing remaining to fix for this issue would be to update atom-ide-purescript to run spago sources

@starper
Copy link

starper commented Jan 16, 2019

update atom-ide-purescript to run spago sources

I've created an issue for it

@Dretch
Copy link
Collaborator Author

Dretch commented Jan 19, 2019

#66 is in (thanks!), so it is now possible to run spago build -- --json-errors. I have found a new problem, however: spago build dies unless the HOME environment variable is defined (Dhall needs it to find its cache directory), but purescript-language-server does not pass HOME through (see nwolverson/purescript-language-server#44).

I also have not managed to get go-to-definition to work, and I don't know why (my purescript source glob is set to src/**/*.purs, bower_components/**/*.purs, .spago/*/*/src/**/*.purs).

@f-f
Copy link
Member

f-f commented Jan 19, 2019

@Dretch oh wow, I think this is a Dhall bug, opened dhall-lang/dhall-haskell#788

On the other hand, if Dhall is not able to find a place to cache imports, your package-set will have to be fetched from GitHub every time the atom plugin tries to build (so e.g. you won't be able to build while offline), so you would really want the plugin to pass HOME to subcommands

@Gabriella439
Copy link

Gabriella439 commented Jan 19, 2019

Yeah, that's a Dhall bug which I'm fixing.

Note that XDG_CACHE_HOME is even better environment variable to set if that is an option since it doesn't have to be tied to your home directory. The "${HOME}/.cache" directory is only used as a fallback if "${XDG_CACHE_HOME}" is not defined. See https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html for more details

@megamaddu
Copy link

megamaddu commented Jan 20, 2019

spago build -- --json-errors also seems to be adding spago: Failed to build: 1 to the end of the JSON which prevents VSCode's psc-ide integration from parsing it.

@f-f f-f mentioned this issue Jan 20, 2019
@f-f
Copy link
Member

f-f commented Jan 20, 2019

@spicydonuts fix for that should be up in #82 (here)

Though I'm not sure how much of a fix this is. E.g. in case of build success we do output "Build successful" after the purs output. Is that fine? (I mean in this line)
And in general before the json output there is usually a bunch of other log lines related to installing/building, do they confuse the plugins as well? (i.e. should all the output be json or it's fine if it's just at the end of the build log?)

@megamaddu
Copy link

I'm not 100% sure but I thought the entire output had to be parseable as JSON

@f-f
Copy link
Member

f-f commented Jan 21, 2019

This is weird, because here in the language-server (on which the VSCode and the atom plugin are based on) there's an Array.partition that sieves for lines that look like the json that the compiler emits and tries to parse that, and logs the lines it cannot parse as json.

So even the current version should work 🤔

@spicydonuts could you try to install spago from this branch and see if it fixes the issue?

@f-f
Copy link
Member

f-f commented Jan 22, 2019

@Dretch the issue of spago dying when HOME is not set is now fixed in master, I'll make a release today or tomorrow if it's ok (I'd like to figure out this VSCode thing so we ship all the fixes together)

@nwolverson
Copy link
Collaborator

RE language-server and junk in the JSON output - no, it should only matter that the JSON output is (unprefixed) on a single line on stderr, as per compiler or pulp output, both of which already include junk lines too. Nate did have some issue on a "real world" project which we couldn't reproduce so maybe there is some issue around there though.

(On the other points in this integration - I'll need to find some time but will address the spago sources command & checking env variables are passed through)

@f-f f-f added this to the 1.0 milestone Feb 6, 2019
@f-f
Copy link
Member

f-f commented Feb 10, 2019

@Dretch @nwolverson I think with the latest version of purescript-language-server (0.12.7) and the latest version of spago (0.6.4) this issue can be considered fixed, right?

@Dretch
Copy link
Collaborator Author

Dretch commented Feb 12, 2019

@f-f I think a new release of atom-ide-purescript is needed before this is considered completely fixed.

@JordanMartinez
Copy link
Contributor

Just checking in on the status of this issue.

It's been 22 days since the last comment and I don't see a new release for atom-ide-purescript. I'm assuming this issue hasn't been fixed yet, correct?

@nwolverson
Copy link
Collaborator

I just pushed a release of atom-ide-purescript (finally)

@f-f
Copy link
Member

f-f commented Mar 7, 2019

Wonderful! It looks like we can close this one, but feel free to comment here if there are more issues and this needs to be reopened

Thanks a lot @nwolverson, @Dretch and everyone involved! 👏

@f-f f-f closed this as completed Mar 7, 2019
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

No branches or pull requests

9 participants