Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Reload completion on file changes #98

Merged
merged 17 commits into from
May 6, 2018
Merged

Reload completion on file changes #98

merged 17 commits into from
May 6, 2018

Conversation

faustinoaq
Copy link
Member

@faustinoaq faustinoaq commented Apr 30, 2018 •
edited
Loading

This PR does:

Changes

/cc @laginha87

@faustinoaq faustinoaq requested a review from a team April 30, 2018 02:00
@faustinoaq faustinoaq added this to To do in Crystal Tools via automation Apr 30, 2018
@faustinoaq faustinoaq moved this from To do to In progress in Crystal Tools Apr 30, 2018
@faustinoaq
Copy link
Member Author

Relevant commit: 454b93c

@laginha87
Copy link
Contributor

Damn was also working on fixing this one :P
Looks like the right way to go 👍

@faustinoaq
Copy link
Member Author

faustinoaq commented Apr 30, 2018 •
edited
Loading

@laginha87 Yeah, I'm trying to fix #61, although, this PR doesn't work yet. The graph is not updated on .rebuild and I don't know why 😅

Maybe I'm checking the wrong file? on if file == filename ?

@laginha87
Copy link
Contributor

that bit is a bit overkill you should be able to just run process_requires on the filename
Also you need to update the method_db for the open file for it to work. For now a quick test you can do for the dependancy graph part is close and open the file.

@faustinoaq
Copy link
Member Author

faustinoaq commented Apr 30, 2018 •
edited
Loading

screenshot_20180430_094118

@laginha87 Working now! 🎉 And is resilient to errors in code as well ^ ✨

@faustinoaq faustinoaq mentioned this pull request Apr 30, 2018
Closed
@faustinoaq faustinoaq requested a review from a team May 4, 2018 00:15
@faustinoaq
Copy link
Member Author

faustinoaq commented May 5, 2018 •
edited
Loading

@crystal-lang-tools/scry Can I get another review here? 😅

bew
bew approved these changes May 5, 2018
View reviewed changes
src/scry/workspace.cr Outdated
def initialize(root_uri, process_id, max_number_of_problems)
@root_uri = root_uri
@process_id = process_id
@max_number_of_problems = max_number_of_problems
@open_files = {} of String => {TextDocument, Completion::MethodDB}
@dependency_graph = Completion::DependencyGraph::Graph.new
@lookup_path = ENV["CRYSTAL_PATH"].split(":") + [@root_uri]
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: Remember: If you're doing + [expr] you're having 2 new arrays!

Copy link
Member Author

Choose a reason for hiding this comment

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

What about ENV["CRYSTAL_PATH"].split(":") << @root_uri ?

@faustinoaq faustinoaq merged commit 4243c3f into master May 6, 2018
Crystal Tools automation moved this from In progress to Done May 6, 2018
@faustinoaq faustinoaq deleted the fa/reload-completion branch May 6, 2018 06:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laginha87 laginha87 laginha87 approved these changes

@bew bew bew approved these changes

Assignees
No one assigned
Labels
enhancement help wanted
Projects
Crystal Tools
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@faustinoaq @laginha87 @bew