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

Initial project system #44

Merged
merged 20 commits into from
Oct 15, 2014
Merged

Initial project system #44

merged 20 commits into from
Oct 15, 2014

Conversation

sharwell
Copy link
Member

Fixes #9

Remaining work

  • The branch includes many unnecessary features in the project system property pages, and/or items which do not make sense for Dart projects
  • Need to disable the scanning, and only enable Dart analysis for non-.dartproj projects if/when a Dart file which is in that project gets opened (Cannot scan projects using DTE #38)
  • Project templates
    • Identify reasonable project templates for Dart projects (help wanted)
    • Implement the project templates
  • Item templates
    • Identify reasonable item templates for Dart projects (help wanted)
    • Implement the item templates

@sharwell sharwell changed the title [WIP] Project system Initial project system Oct 14, 2014
@jrote1
Copy link

jrote1 commented Oct 15, 2014

My Opinions (I may be able to help in the future but currently I am very busy):

Project Templates:

  • Web Appication:
    • web/{project name}.dart
    • web/{project name}.html
  • Command Line:
    • bin/{project name}.dart
  • Package
    • lib/{project name}.dart
    • test/
  • For all templates a standard pubspec.yaml should be added (and all files should have default content)

File Templates:

  • Class
    • If possible Class whie when file is called my_first_class.dart the content would be

      class MyFirstClass{
      
      }
      

@sharwell
Copy link
Member Author

@jrote1 I created a template for each of the items you mention. If you have any questions about their behavior or suggestions to change the default content, please don't hesitate to add comments. I'm not a Dart programmer so I just threw them together.

@DanTup
Copy link
Contributor

DanTup commented Oct 15, 2014

Starting to look good!

Some feedback... Much of which you're probably aware of and is just incomplete; I'm just dumping everything I see that I think we should do/discuss for completion. Happy to do some of these myself if you want; but don't want to step on your toes and create lots of conflicts right now!

General

  • Default names should be lowercase_with_underscores for both project name (inc folder)
  • We shouldn't create two folders (solution and project) if possible, just one level (ideally, should be solutionless; but not sure how this works in VS) User can opt not to create solution folder
  • Default puspec.yamls shouldn't include intl (and console app shouldn't include browser)
  • .dartproj currently lists all files; will this work as wildcard or something? We should support a non-VS user and a VS-user working on the same project well (we can probably fix this up on load in VS, though not having it in the commits for every new file would be nice) Moved to Don't list all files in the .dartproj file #59
  • pubspec.yaml currently crashes when trying to open in the editor Cannot repro
  • there's a bin\Debug folder created; is it possible to avoid this? (it's a bit weird because bin is where scripts go!)

Solution explorer

Project Properties

@sharwell
Copy link
Member Author

General

Default names should be lowercase_with_underscores for both project name (inc folder)

Fixed in 35f4900

We shouldn't create two folders (solution and project) if possible, just one level (ideally, should be solutionless; but not sure how this works in VS)

💡 Uncheck "Create directory for solution" when creating the project. Avoiding the use of a solution file and/or a project file is not a realistic goal for the upcoming release.

Default puspec.yamls shouldn't include intl (and console app shouldn't include browser)

Fixed in 6729271

.dartproj currently lists all files; will this work as wildcard or something? We should support a non-VS user and a VS-user working on the same project well (we can probably fix this up on load in VS, though not having it in the commits for every new file would be nice)

Supporting this is mostly straightforward.

  1. ❓ Should all files be included, or only .dart files?
  2. ❓ Should all files under all folders be included, or only files in bin, lib, test, and web?

pubspec.yaml currently crashes when trying to open in the editor

❗ Cannot reproduce.

there's a bin\Debug folder created; is it possible to avoid this? (it's a bit weird because bin is where scripts go!)

Fixed in aaac8c2

@DanTup
Copy link
Contributor

DanTup commented Oct 15, 2014

❓ Should all files be included, or only .dart files?
❓ Should all files under all folders be included, or only files in bin, lib, test, and web?

Generally, we should show the filesystem as-is. Since there's no "build" when executing, the concept of "included" and "excluded" files wouldn't mean anything. All tooling just behaves on the files.

There is one possible exception to this relating to the "References" node which I thnk can replace all of the pakages folders (we'd need to rename it to packages); but that's detailed in #53 so for now; I think we should just render everything.

pubspec.yaml currently crashes when trying to open in the editor

❗ Cannot reproduce.

Also cannot repro now. Will raise this as a new issue if it comes up again!

@sharwell
Copy link
Member Author

Solution Explorer

File properties are redundant; CopyToOutput don't make sense, and the equiv of Custom Tool would be a Dart Transformer which would be configured in pubspec.yaml

Copy to Output Directory removed in 9a4e145. I prefer to defer the custom tool issue.

Folder properties have Build Action with options of Folder, Source Folder, TestSourceFolder, is this deliberate, or something to ditch?

Fixed in 74ef0d4

References folder - currently has NuGet menu options. For now, I think we should ditch this; but I'll detail this in #53

Injected by the NuGet extension, generally outside the control of this extension.

I added a DartVS logo to the repo; might be nice to use it as the project node icon (it's the Dart icon colours in the VS logo shape), keeping it slightly different to the normal Dart icon on .dart files

❗ Needs to be a .ico file to use for this. Currently I only see a .png.

Lots of context menu items in solution tree are not relevant

❓ Can you provide a list of specific items that you want to see addressed?

@DanTup
Copy link
Contributor

DanTup commented Oct 15, 2014

Injected by the NuGet extension, generally outside the control of this extension.

Is this done by name? For now, I think we should hide References, but for #53 I think we should rename it packages.

Needs to be a .ico file to use for this. Currently I only see a .png.

I've converted this; should be in repo now.

Can you provide a list of specific items that you want to see addressed?

I think most of those Isaw where actually on the Solution; so we probably can't change them. On the project I think we should hide Build, Rebuild, Clean though (and ideally NuGet, but sounds like that might not be trivial).

@sharwell
Copy link
Member Author

Project Properties

Package name is stored in pubspec.yaml. We should either sync these, or remote from properties (I think syncing would be nicest; treating pubspec.yaml as master)

This is a bit complicated. I recommend leaving the current behavior in place right now and addressing this as an independent issue after the initial project system is merged.

VM/Output type are not relevant

❓ Could output type could be relevant if the build produces redistributable packages of some form (e.g. "bundles"), or a dart2js output? What about using target virtual machine as and indicator of Dartium or some other runtime?

Startup Object should ideally let you pick any file from the file system (in this project, ofc)

I recommend leaving the current behavior in place right now and addressing this as an independent issue after the initial project system is merged.

Option for which browser to launch is needed; since most dev will be done with Dartium. When selecting Dartium; it's also important to pass --user-data-dir so not sure if we should use environment variables (to avoid configuring every project)? (DARTIUM, DARTIUM_PROFILE?)

The .dartproj.user file is the place to store this information. We can address this as a separate issue after the initial project system is merged, and I can outline the specific manner in which items are read from / written to this file.

Configuration/Platform: Probably not relevant, though Configuration might be handy to switch between builds (Dartium in Debug, Chrome in Release), unless there's a better way we can let the user choose the browser

Platform is relevant at least for console applications, since Dart is available in both x86 and x64 flavors. Could also be used to switch between specific browsers (e.g. "Chrome" could be considered a platform). Need to compare this use case to other project types.

Most stuff on the Build tab isn't currently relevant; I don't think there's value in errors-as-warnings here, as there's not really such thing as a "build" and an important decision with Dart is that warnings don't stop you executing. There is such a thing as a "build", but it's more like a "package for deployment", so I'm not sure if we should tie it to VS's normal build structure (we don't want to kick it off for hitting Ctrl+Shift+B for ex!). I'll flesh #17 with more details about how Run/Debug/Build(Package) will work later

Disabled debug commands in 70f4637.

Build events - I think maybe pub transformers would be a better way for people to do stuff on build(package), so maybe we should remove this

I recommend leaving the UI in place until we know for sure what the Build command does.

Debug - Currently this is a big unknown, so I think hide it for now, and we'll flesh #28 out when Google have implemented this in the Analysis Service

See above.

When I changed build config from Any to Debug/Release, it reported The target "GetFrameworkPaths" does not exist in the project. in errors window

Fixed in 306c30b.

@sharwell
Copy link
Member Author

I've converted this; should be in repo now.

Applied to project templates in 8ee45bf

@DanTup
Copy link
Contributor

DanTup commented Oct 15, 2014

This is a bit complicated. I recommend leaving the current behavior in place right now and addressing this as an independent issue after the initial project system is merged.

I've added #57 to look at this later.

❓ Could output type could be relevant if the build produces redistributable packages of some form (e.g. "bundles"), or a dart2js output? What about using target virtual machine as and indicator of Dartium or some other runtime?

pub build does take a mode which can be release (which enabled minification, and strips .dart files); though I don't know if there are any options to give the user besides picking between Debug/Release (and this only applies to pub build which is effectively "build deployment").

Using Dartium/Chrome/etc. doesn't really feel right in "target machine", as it's not really a target, nothing changes; it's just the browser to launch. In normal ASP.NET web apps, there's a browser-selector; which would be ideal to use for this; but I don't know if it's reusable. If not, we do need browser selection, though I don't think it should be named "target".

Startup Object should ideally let you pick any file from the file system (in this project, ofc)

I recommend leaving the current behavior in place right now and addressing this as an independent issue after the initial project system is merged.

Yeah, this should be dealt with as part of #17.

Option for which browser to launch is needed; since most dev will be done with Dartium. When selecting Dartium; it's also important to pass --user-data-dir so not sure if we should use environment variables (to avoid configuring every project)? (DARTIUM, DARTIUM_PROFILE?)

The .dartproj.user file is the place to store this information. We can address this as a separate issue after the initial project system is merged, and I can outline the specific manner in which items are read from / written to this file.

Yeah, this should also be part of #17.

Platform is relevant at least for console applications, since Dart is available in both x86 and x64 flavors.

This will be handled by the users choice of SDK; it's not really a "target". When executing, we can either use DART_SDK or assume pub is in PATH, we can't switch between x64/x86 with knowledge of only one SDK.

@DanTup
Copy link
Contributor

DanTup commented Oct 15, 2014

Disabled debug commands in 70f4637.

My comment was actually about the options on the Debug tab rather than the commands! :)

DanTup added a commit that referenced this pull request Oct 15, 2014
@DanTup DanTup merged commit de47586 into DartVS:master Oct 15, 2014
@sharwell sharwell deleted the project-system branch October 15, 2014 20:39
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.

Dart Project Type
3 participants