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

Copy the file name when creating a gist #197

Closed
georgebearden opened this issue Jan 14, 2016 · 7 comments
Closed

Copy the file name when creating a gist #197

georgebearden opened this issue Jan 14, 2016 · 7 comments

Comments

@georgebearden
Copy link

To further refine the Gist work being done in #194 it would be nice to set the default file name of the gist being created to the name of the file the text is being selected out of. Here is a working snippet that has been tested on a single-pane and multi-pane text editor.

var dte = GetService(typeof (DTE)) as DTE;
if (dte == null)
    return;

var fileNameWithPath = dte.ActiveDocument.FullName;
var fileName = Path.GetFileName(fileNameWithPath);
@shana
Copy link
Contributor

shana commented Jan 14, 2016

@georgebearden Just FYI, Services.DTE is available for getting at these interfaces.

@georgebearden
Copy link
Author

Hi @shana, I just went to implement this feature and ran into one issue. Trying to reference DTE or DTE2 from the GitHub.App project leads to the following compile-time error.

error CS1762: A reference was created to embedded interop assembly 'EnvDTE, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' because of an indirect reference to that assembly created by assembly 'GitHub.Exports, Version=1.0.16.3, Culture=neutral, PublicKeyToken=null'. Consider changing the 'Embed Interop Types' property on either assembly.

I don't want to go changing assembly info without knowing what is going on, but here is the code snippet we would need to add to the GistCreationViewModel's constructor:

var fullName = VisualStudio.Services.Dte2?.ActiveDocument?.FullName;
FileName = fullName != null ? Path.GetFileName(fullName) : Resources.DefaultGistFileName;

Don't you hate it when you think some feature is going to be super easy and then you run into these kinds of issues.

One workaround (but prob. not MVVM compliant) would be to set the FileName like we do above, but instead of settings in the GistCreationViewModel which lives in the GitHub.App project we can set it in the code-behind of the GistCreationView as it lives in the GitHub.VisualStudio project which already has references to EnvDTE and EnvDTE80.

@shana
Copy link
Contributor

shana commented Jan 18, 2016

Oh yes, this happens often when pulling in a direct reference to EnvDTE.

Warning: Wall of text.

EnvDTE includes embedded interop types, which means that the assembly is wrapping a native com type in a .net type and it provides the complete information of the native type (basically the size of it, that's what really counts), and allowing others to save this information when they use the .net wrappers. This allows a user assembly to use a .net type that maps into a com type even if the version of the assembly where that type comes is different at runtime.

COM types are made in a way that they're backwards-compatible and have type information built in (meaning that a type can be queried about what it can be cast to at runtime). The interfaces they expose never change once public, and are mapped to classes whose vtables get methods added to the end of them. A class conforming to these rules can always grow with more methods without breaking existing interfaces that map them, and new interfaces are added that contain more methods.

When you request DTE, depending on your VS version you'll get the instance from one of the 5 or more different versions of the assembly it lives in (VS ships a different one every release). The instance you get is a "filtered" interface to a (potentially) much larger object (depending on which assembly it came from), and it can do that because it knows what the original size of the type was, back when only DTE existed, because it gets recorded during compilation (by embedding the type information). This is, incidentally, why there's so many ISomething ISomething2 ISomething3 interfaces in the VS world, and also, haha, explains the naming convention of the Mozilla Runtime API (yes, COM exists in Linux and very successfully so. Oh the irony)

When you add a reference to EnvDTE (or any other assembly that is marked as exporting type information), the default is to embed that type information into your assembly via the Embed Interop Types option, because allows you to have no dependencies at runtime. If the Embed Interop Types option is false, type information is not embedded in the caller and the runtime instead uses an external assembly that is considered the authoritative source of information for the type (the PIA, primary interop assembly). TLDR, com types are either linked at compile time or they're referenced and resolved at runtime.

Anyways, the problem is the compiler will only do it if every assembly you reference that references EnvDTE also has Embed Interop Types set to true. If any assembly referenced directly or indirectly has it set to false, then the compiler refuses to embed the types on your assembly, because it is already forced to reference the PIA at runtime (due to one of the assemblies having Embed Interop Types set to false), which will cause two copies of the same type to be around when the code runs, and that would be bad. In this case, GitHub.App references GitHub.Exports which references Microsoft.TeamFoundation.Git.Provider which references EnvDte with Embed Interop Types set to false.

So you hit this amazingly useless compiler error and go wtf for a while and get to read this wall of text probably thinking how did you get into this mess in the first place and wondering where that bottle of whiskey went off to.

@shana
Copy link
Contributor

shana commented Jan 18, 2016

Anyways, we're having a real problem passing information from the outside to the view models. This would not be an issue if there was a simple protocol for passing action-specific data to the UIController for seeding the view/viewmodels. The filename should be passed to the viewmodel as data for it to use, the same way we pass the IConnection so it knows what account to do the action on. That's the real problem that you're hitting IMO.

As a workaround, one of the other PRs has a MEF IActiveDocumentSnapshot that exposes file name information without forcing this DTE link issue. Either that, or have a method in GitHub.Exports return the filename, and mark it with a TODO for sorting out later.

I don't want to pollute GitHub.App with too much vs-specific stuff, it makes it harder to test and is generally meh. At some point we'll probably end up with a library for small-and-useful VS-specific functionality, so the less we spread VS stuff around the better.

@georgebearden
Copy link
Author

Thank you for the explanation! I think the best thing to do is use the IActiveDocumentSnapshot from #191. If we want the this feature in the release and #191 wont be in it I can do one of the following based on your notes above

  1. Add a static method off of Services.cs
  2. Add a method to the ISelectedTextProvider
  3. Set the FileName of the gist in the code behind of the GistCreationView

Let me know if any of these strike you as least bad 😄

@shana
Copy link
Contributor

shana commented Feb 1, 2016

@georgebearden Hey hey! I was away last week on a trip, so that's why I didn't get back to you before (no internets!). Adding a static method in Services.cs sounds about right for now, we can easily move it around once we know where things should be.

georgebearden pushed a commit to georgebearden/VisualStudio that referenced this issue Feb 2, 2016
shana added a commit that referenced this issue Feb 3, 2016
Copies the file name of the active document to the gist popup, #197
@paladique
Copy link
Contributor

This was done here: #214

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants