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.

Sign up for GitHub

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

Jump to bottom

[psc-ide] Goto definition #2210

Closed
wants to merge 2 commits into from
Closed

[psc-ide] Goto definition #2210

wants to merge 2 commits into from

Conversation

kritzcreek
Copy link
Member

Works surprisingly well in my first tests. I still need to add a few integration tests, and I think the API could use some polish.

@michaelficarra michaelficarra mentioned this pull request Jun 27, 2016
Closed
@kritzcreek
Copy link
Member Author

kritzcreek commented Jun 28, 2016 •
edited

Allright I'm happy now :) One thing that concerns me, is that I can't make the defaults assumption of src/**/*.purs and bower_components... for the sourcefiles, since we are in the compiler. This makes starting the server a bit cumbersome.

EDIT: The test failure on windows looks genuine. It's probably about some path separator, I'll take a look later

@kritzcreek kritzcreek changed the title [WIP][psc-ide] Goto definition [psc-ide] Goto definition Jun 28, 2016
@kritzcreek
Copy link
Member Author

Allright :) Ready for review from my side. The Windows CI is fixed and OS X are failing as always...

paf31
paf31 reviewed Jul 2, 2016
View reviewed changes
src/Language/PureScript/Ide/SourceFile.hs
-- | Given a surrounding Sourcespan and a Declaration from the PS AST, extracts
-- definition sites inside that Declaration.
extractSpans
:: P.SourceSpan -- ^ The surrounding span
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might like to move the comments to the next line. The ^ is meant to point upwards to the code above, I believe.

@paf31
Copy link
Contributor

paf31 commented Jul 2, 2016

Looks good! So, this adds position info for type queries, but no new commands for general name queries yet, correct?

@kritzcreek
Copy link
Member Author

Yep. I'm planning to turn the type command into something like ghci's :info to adress #1136 and #1053.

@paf31
Copy link
Contributor

paf31 commented Jul 2, 2016

Could you please create an issue for this, so we have a place for comments etc.?

@paf31
Copy link
Contributor

paf31 commented Jul 2, 2016

I'd like to know why Travis keeps failing on MacOS. Maybe we should just disable those builds until we can figure out what's going on.

@kritzcreek
Copy link
Member Author

I've listed my current backlog for psc-ide in #2164 under Planned Features. I'll turn that into a checklist to show the current progress.

@coveralls
Copy link

coveralls commented Jul 2, 2016 •
edited

Coverage Status

Coverage decreased (-0.3%) to 57.176% when pulling 23ed63a on kRITZCREEK:psc-ide-go-to-definition into 7174a75 on purescript:master.

@kritzcreek
Copy link
Member Author

Thanks for taking a look :) It would be nice if there was another way than me creating GIFs to showcase the new functionalities... I should either learn to hack the atom plugin so that you can try things out, or you could install a version of emacs, just to try out how the new stuff works.

I thought about making a little online playground with ace editor backed by a psc-ide instance to be able to play and test new features but I'll need to think about it a bit more, especially how that's supposed to work on the per-file level :)

@kritzcreek
Copy link
Member Author

Let me know if you want me to squash.

@kritzcreek kritzcreek mentioned this pull request Jul 3, 2016
Merged
@kritzcreek
Copy link
Member Author

rebased and cleaned up in #2215

@kritzcreek kritzcreek closed this Jul 5, 2016
@kritzcreek kritzcreek deleted the psc-ide-go-to-definition branch July 9, 2016 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status: ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kritzcreek @paf31 @coveralls