Allow using statements to point to live package code #562

ZacLN · 2020-02-26T23:24:57Z

Partner of julia-vscode/StaticLint.jl#103

The key question at this point is what files we will allow/determine to be packages. One approach would be to limit it to workspace folders that have the correct structure (i.e. a folder X with src/X.jl within it)

codecov · 2020-02-26T23:28:20Z

Codecov Report

Merging #562 into master will decrease coverage by 0.04%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##           master     #562      +/-   ##
==========================================
- Coverage   28.46%   28.42%   -0.05%     
==========================================
  Files          23       23              
  Lines        1686     1706      +20     
==========================================
+ Hits          480      485       +5     
- Misses       1206     1221      +15

Impacted Files	Coverage Δ
src/requests/completions.jl	`0% <0%> (ø)`	⬆️
src/languageserverinstance.jl	`41.46% <35.71%> (-1.19%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8faa95...569b653. Read the comment docs.

ZacLN · 2020-03-14T20:19:08Z

julia-vscode/julia-vscode#1023

davidanthoff · 2020-05-03T11:03:26Z

Hm, it seems to me that we generally need to resolve this via the active environment, right? Whereas this just seems to be based on file system structure?

So I think if we see a using statement (that is not a relative path type thingy), then we look into the active env. If the package is deved and the path to the deved package is in our workspace, then we use the live CSTParser tree to resolve symbols, otherwise we use what we get from SymbolServer, right?

ZacLN · 2020-05-04T07:00:30Z

This is just the internal handling of this (not the env handling part we discuss the other day) and in particular looks to make available the 'main' package when editing it is being edited - this is not information explicitly held in the Project.toml. I've tightened up the code that decides whether to add a 'live' package to reflect this.

davidanthoff · 2020-05-06T14:16:47Z

I think I still don't fully understand this :)

Lets say someone opens the root folder for package Foo.

Inside the src folder we would never expect a using Foo statement, right? Only include statements to assemble everything together. So I assume that this here is not needed when a user edits the files in src, right?

For files outside of src, it seems to me that we should always and only go through an environment to resolve using X statements. Right now the UI is such that if I open the folder for package Foo, and my active env does not have Foo deved, I get a warning that I should open an env that contains Foo. So the model we have right now is that if someone opens a package folder, then they should have an env active where that package is deved, and then resolving using Foo from folders outside of src should go via the enviornment, right?

I think right now I just don't understand the scenario where we should try to resolve this purely pased on file location.

ZacLN · 2020-05-06T16:18:05Z

inside the src folder we would never expect a using Foo statement, right?

Unfortunately we do, this seems silly.

Keeping with your example above, if I'm editing Foo I think I want all references to Foo within that context (i.e. the package directory) to point to what I'm editing, not a cached version. If, for example, I'm writing tests I'd like to be able to refer to functions/features I've just added.

ZacLN · 2020-05-08T15:11:30Z

Continuing this - isn't this what we discussed, the cache should only be used for code that is outside the current workspace?

ZacLN · 2020-06-22T17:14:05Z

bump @julia-vscode/core I think this could do with some discussion

davidanthoff

We clearly need that kind of thing, but I'm wondering whether a) we should really do this pre Juliacon, and b) whether we should maybe first fix our env handling and then do this? Not sure, to be honest.

davidanthoff · 2020-06-25T04:30:30Z

src/languageserverinstance.jl

@@ -40,6 +40,7 @@ mutable struct LanguageServerInstance
    symbol_store::Dict{Symbol,SymbolServer.ModuleStore}
    symbol_extends::Dict{SymbolServer.VarRef,Vector{SymbolServer.VarRef}}
    symbol_store_ready::Bool
+    workspacepackages::Dict{String,Document}


This should probably have UUIDs as the key, right? I think we are generally not identifying packages by their UUID right now, but probably should to get this right.

davidanthoff · 2020-06-25T04:31:57Z

src/languageserverinstance.jl

+    # Add possible workspace packages
+    path = uri2filepath(uri._uri)
+    for wk_folder in server.workspaceFolders
+        if startswith(path, wk_folder) && normpath(wk_folder) == normpath(server.env_path)


I don't fully understand the logic here. Is the idea that only if one has activated the package as the env, will any of this work?

davidanthoff · 2020-06-25T04:32:31Z

src/languageserverinstance.jl

+            fname = splitext(last(sub_path))[1]
+            if sub_path[end-1] == "src" && sub_path[end-2] == fname
+                @info "Setting $path as source of 'live' package"
+                server.workspacepackages[fname] = doc


It seems to me that we need to look for a Project.toml and then at its content to decide whether something is really a package or not.

davidanthoff · 2020-06-25T04:35:07Z

src/requests/completions.jl

@@ -414,6 +421,11 @@ function import_completions(doc, offset, rng, ppt, pt, t, is_at_end ,x, CIs, ser
                    end
                end
            else
+                for (n,doc1) in server.workspacepackages
+                    if startswith(n, t.val) && StaticLint.has_workspace_package(server, n)


I think this is where we need to go via the env info: Say the code has using Foo, then we need to look in the env what UUID Foo corresponds to, then we look in the server.workspacepackages that is indexed by UUID whether there is a package with that UUID. But we probably shouldn't match by name, because the names don't have to be the same if the package is loaded via an env (and not LOAD_PATH).

davidanthoff · 2021-07-26T18:14:19Z

@ZacLN I'd say we can try to get that in soon after Juliacon? Is the next step that you resolve merge conflicts, and then we review properly?

davidanthoff

@ZacLN bump, I think next step is to resolve merge conflicts here.

davidanthoff · 2021-08-26T15:51:27Z

And now we need to get tests to pass :)

initial commit

8d99754

ZacLN added this to the Next milestone Feb 26, 2020

ZacLN added 3 commits March 13, 2020 13:04

Merge branch 'master' into workspacepackages

5f82d97

wip

c1a51ac

remove doc refs from workspacepackages on deletedoc

569b653

ZacLN mentioned this pull request Apr 27, 2020

0.15.x release planning julia-vscode/julia-vscode#1112

Closed

ZacLN added 4 commits April 27, 2020 22:40

Merge branch 'master' into workspacepackages

34bfcba

fix some completions

ebbe86a

Merge branch 'master' into workspacepackages

7507da4

Merge branch 'master' into workspacepackages

96806ab

ZacLN mentioned this pull request May 2, 2020

Allow using statements to access packages loaded within workspace julia-vscode/StaticLint.jl#103

Merged

1 task

tighten live package identification

a0c3147

davidanthoff assigned ZacLN May 29, 2020

davidanthoff added the enhancement label May 29, 2020

davidanthoff modified the milestones: Next extension minor release, Triage May 29, 2020

davidanthoff self-requested a review May 29, 2020 00:22

davidanthoff reviewed Jun 25, 2020

View reviewed changes

ZacLN mentioned this pull request Jul 6, 2020

A few issues julia-vscode/StaticLint.jl#202

Open

davidanthoff mentioned this pull request Aug 1, 2020

Roadmap to BLISS #812

Open

9 tasks

davidanthoff mentioned this pull request Jul 26, 2021

Disable workspace packages for now julia-vscode/StaticLint.jl#301

Merged

davidanthoff removed this from the Triage milestone Jul 26, 2021

davidanthoff modified the milestones: Next Minor, Next Patch Jul 26, 2021

davidanthoff requested changes Aug 23, 2021

View reviewed changes

Merge branch 'master' into workspacepackages

e547cdc

Merge branch 'master' into workspacepackages

ce1f4c1

davidanthoff modified the milestones: Next Patch, Backlog Mar 27, 2022

Merge branch 'master' into workspacepackages

95d2d81

davidanthoff removed this from the Backlog milestone Oct 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow using statements to point to live package code #562

Allow using statements to point to live package code #562

ZacLN commented Feb 26, 2020 •

edited

codecov bot commented Feb 26, 2020 •

edited

ZacLN commented Mar 14, 2020

davidanthoff commented May 3, 2020

ZacLN commented May 4, 2020

davidanthoff commented May 6, 2020

ZacLN commented May 6, 2020

ZacLN commented May 8, 2020

ZacLN commented Jun 22, 2020

davidanthoff left a comment

davidanthoff Jun 25, 2020

davidanthoff Jun 25, 2020

davidanthoff Jun 25, 2020

davidanthoff Jun 25, 2020

davidanthoff commented Jul 26, 2021

davidanthoff left a comment

davidanthoff commented Aug 26, 2021

Allow using statements to point to live package code #562

Are you sure you want to change the base?

Allow using statements to point to live package code #562

Conversation

ZacLN commented Feb 26, 2020 • edited

codecov bot commented Feb 26, 2020 • edited

Codecov Report

ZacLN commented Mar 14, 2020

davidanthoff commented May 3, 2020

ZacLN commented May 4, 2020

davidanthoff commented May 6, 2020

ZacLN commented May 6, 2020

ZacLN commented May 8, 2020

ZacLN commented Jun 22, 2020

davidanthoff left a comment

Choose a reason for hiding this comment

davidanthoff Jun 25, 2020

Choose a reason for hiding this comment

davidanthoff Jun 25, 2020

Choose a reason for hiding this comment

davidanthoff Jun 25, 2020

Choose a reason for hiding this comment

davidanthoff Jun 25, 2020

Choose a reason for hiding this comment

davidanthoff commented Jul 26, 2021

davidanthoff left a comment

Choose a reason for hiding this comment

davidanthoff commented Aug 26, 2021

ZacLN commented Feb 26, 2020 •

edited

codecov bot commented Feb 26, 2020 •

edited