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

Workspace peek #191

Merged
merged 24 commits into from
Aug 30, 2017
Merged

Workspace peek #191

merged 24 commits into from
Aug 30, 2017

Conversation

enricosada
Copy link
Contributor

@enricosada enricosada commented Aug 2, 2017

ref #181

this PR add a workspacePeek command who return info about interesting stuff in a directory.
this is a really fast command, just do a peek (up to a deep level, passed as option in the command) so user/editor can later choose what use.

This allow client to open big repo, with mixed solution too, and choose only a subset to initialize.
Otherwise client atm just try to load all (long and useless) or the one the client think is related to a .fs file opened (can be wrong, for example if there are more or is imported with relative paths)

client can also pass the directories to exclude from search, useful for the like of .git , paket-files etc

Ideal flow:

  • client ask fsac for workspacePeek, who return the list of solutions and fsproj in directory
  • based on client settings or user choice, a workspace is choosen (sln or directory)
  • for all fsproj in the workspace, a project command is sent, to initialize only these

Next work (not this PR) is maybe a workspaceLoad, who accept to choice, to initialize a whole solution or list of fsprojs

Big items:

  • solution: *.sln are parsed (list of solution folder, projects, etc) and full structure graph is returned. fsproj are not yet parsed to initialize FCS in this step, for that continue to be needed the project command.
  • directory: current directory with all fsproj paths

I excluded the .fsx because while interesting sort of, can be a lot, and just noise atm.

todo:

  • http based
  • stdio based
  • test

@enricosada
Copy link
Contributor Author

enricosada commented Aug 2, 2017

code has microsoft.msbuild.* namespace because i stealed the code from https://github.com/microsoft/msbuild repo, but DOESNT use msbuild automation and is zero deps

@enricosada
Copy link
Contributor Author

published the sln parsers as package, from https://github.com/enricosada/sln

@enricosada enricosada changed the title [WIP] Workspace peek Workspace peek Aug 8, 2017
@enricosada
Copy link
Contributor Author

@rneatherway @kjnilsson @Krzysztof-Cieslak this PR is done for me, asking for review/feedback.

@@ -5,6 +5,7 @@ nuget Argu 3.7
nuget FSharp.Compiler.Service 11.0.9 framework: >= net45
nuget FSharp.Compiler.Service.ProjectCracker 11.0.9
nuget Dotnet.ProjInfo 0.7.4
nuget Sln 0.2.0
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to pin version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is not 1.0 yet, so i can publish new version and i dont guarantee the api compatibility between minor.
just to avoid issues for others, while updating deps.

WorkspacePeekFound.Solution { WorkspacePeekFoundSolution.Path = p; Items = items; Configurations = [] }

let data = { WorkspacePeekResponse.Found = found |> List.map mapInt }
serialize { Kind = "project"; Data = data }
Copy link
Member

Choose a reason for hiding this comment

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

We try to use different Kind for each response, project is used by project command

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy paste, sigh, good catch

}

[<RequireQualifiedAccess>]
type Interesting =
Copy link
Member

Choose a reason for hiding this comment

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

Interesting doesn't sound like best name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggestions? cannot think anything else atm.
this is the list of stuff who can or cannot be useful in the dir (for example fsx are not returned by command later)

Copy link
Contributor

Choose a reason for hiding this comment

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

Content or Contents?

|> Some
with _ ->
None
match slnFile with
Copy link
Member

Choose a reason for hiding this comment

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

Option.map ?

}
and [<RequireQualifiedAccess>] WorkspacePeekFoundSolutionItemKindFolder = {
Items: WorkspacePeekFoundSolutionItem list
Files: string list
Copy link
Contributor

Choose a reason for hiding this comment

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

What ends up here rather than in Items?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The files inside a solution folder, in the list the full paths.
Is in another property instead of WorkspacePeekFoundSolutionItem, because a file can be only inside a solution folder, not at root level (solution) like a project

@rneatherway
Copy link
Contributor

Works for me, no objections.

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

Successfully merging this pull request may close these issues.

3 participants