Remove type provider security dialog and use custom icon for type provider assembly reference #448

enricosada · 2015-05-13T14:08:18Z

I was hasty about icon design, sry.

This is a pr to enable a custom icon based of assembly type (normal or type provider), but the custom icon will come from the VS branding team.

by @KevinRansom (see below)

Just to set expectations.
Whilst I think it's interesting to scout ideas, and to produce prototypes, Icons are part of branding and VS branding is not owned by the feature team (us), so the outcome of this effort will likely serve as input to what the VS branding team does.

allykzam · 2015-05-13T14:53:12Z

Cross-posting from #378, my 2¢ is to somehow base the icon off of the F# logos. My initial preference would be to just globally use the blue logo, but it might look better to use the blue logo with Visual Studio's light and blue themes, but to use one of the dark logos with the dark theme.

All three for comparison (the empty space below is the light logo, and is basically white; preview it over something dark):

forki · 2015-05-13T15:01:13Z

@ReedCopsey as an official fsharp.org spokesperson, can you please comment on possible license issues?

enricosada · 2015-05-13T15:09:21Z

16x16 is really small, with fsharp logo ( resized from http://fsharp.org/favicon.ico ) is like

and i dont see dark/light themed icons in current codebase, maybe is easier to start with only one

ihmo the fsharp logo is not the best solution, the logo = fsharp language, not a type provider.
maybe is more meaningful for other stuff

enricosada · 2015-05-13T15:50:10Z

added the placeholder, download e53ce24 to design and test the icon inside visualstudio

the placeholder is last spot in imagelist, and the icon is applied to all assemblies, so any project is ok

KevinRansom · 2015-05-13T17:05:08Z

Just to set expectations.

Whilst I think it's interesting to scout ideas, and to produce prototypes, Icons are part of branding and VS branding is not owned by the feature team (us), so the outcome of this effort will likely serve as input to what the VS branding team does.

Thanks

Kevin

latkin · 2015-05-13T19:32:41Z

+1 to @KevinRansom, we can't just come up with an icon of our choosing. Lots of legal, globalization, and marketing factors to consider. It needs to come from our dedicated art/branding people, or at least re-use an already-approved icon used elsewhere in VS.

forki · 2015-05-13T20:30:49Z

Since we all want to get it in asap we should consider to just change the color of the existing icon in order to formally fulfill the requirement of different icon. The beautification process can be done separately.

enricosada · 2015-05-15T17:13:17Z

waah, these commit are for master branch not fsharp4 😢 i'll fix this tomorrow

Now the assembly reference icon is changed ( using old logic ) only after a build.
I'll enable the other old path for dialog, and use that too for change icon (so icon change after add reference)

I removed pretty much all code/resources, splitted in multiple commit for easier diff

i tried to remove the type providers option page ( Options -> Tools -> Visual F# -> Type Providers ), removing

the type TypeProviderToolsOptionsPage
the ProvideOptionPage vsintegration/src/vs/FsPkgs/FSharp.Project/FS/Project.fs

but no luck, the menu is always there, only contains an error message about page creation.
So now there is the menu, but the page is empty. Can you remove it or we need to add a message?

the refresh is after build

…PartiallyCanonicalized

…WeChecked this global variable was used to save the last parsed file, to find current project. That doesn't work because if the file is a .fsx script, a standalone project is used instead of .fsproj

…rSecurityDialogBlockingUI

enricosada · 2015-05-21T18:26:00Z

removed all unused code, updated comment and cleanup.

tools option page

i removed the tools options page code but the page continue to exists because the VF# extension ( not the EnableOpenSource extension ) add the registry entry HKEY_CURRENT_USER\Software\Microsoft\VisualStudio\14.0FSharp_Config\ToolsOptionsPages\F# Tools\Type Providers .
I think a new build of vf# extension fix this problem /cc @KevinRansom

update assembly icon strategy

now the icon is updated after a build, but not if the project is already up-to-date (for example after reopening of a solution). a new build update the icon, so np.
This remove the global displyDialog, recenteFile global and clean the code alot.

maybe we can remove the old flow or improve it. The flow is:

use fsc code to compile
use a function to publish type provider assemblies path
exit after assemblies are imported, before typecheck

but i need an hint /cc @dsyme @latkin @KevinRansom

latkin · 2015-08-17T22:04:26Z

I'm working on getting this merged. It will be nice to say goodbye to the TP security dialog 👋

latkin · 2015-08-19T02:20:11Z

Worked through this today and came to the following:

Changes in this PR do a fine job of removing the TP dialog, that part is ok
It's simple to remove the options menu page for TP approvals with a small setup change (remove a reg key)
It's not entirely easy to retrofit the old "pop a dialog" codepaths to now mark a reference node as containing a type provider
- TPs can be reliably checked at the point of launching a full build. That part is implemented in this PR.
- But what I would expect as a user, and what's not in this PR, is for the reference node to be marked properly at design-time, right when I add the reference. Making this work reliably is a bit hard*. Without this bit, you have to wait until you do a full build before references are updated.

So I propose we scope this PR down to simply "removing the dialog," ignoring the "mark a reference node as containing TPs" feature for now. That's low-risk and achieves the primary goal of ditching the dialog.

I'll see what I can do to make the marking feature more robust, and open a separate PR for that.

* When the language service compiler encounters a type provider, the current mechanism for tying that back to a particular VS project is GlobalsTheLanguageServiceCanPoke.theMostRecentFileNameWeChecked. It's assumed that whatever project contains the last-processed file must also contain the just-encountered TP reference. From some quick testing, this is actually not a safe assumption - I found that the last-processed file was sometimes in a totally different project. When we were just popping a dialog, it didn't really matter if this was inaccurate, but now it does.

latkin · 2015-08-19T04:06:44Z

If ProcessCommandLineArgsAndImportAssemblies is cheap, then we could make the feature work reliably by simply removing the if actuallyBuild then condition here.

Right now, adding a reference triggers the Compile method, but actuallyBuild is false so we don't get full references computation.

enricosada · 2015-08-20T08:57:37Z

thx @latkin! i think the old code should be axed completly, is a global hack, and need lot of work for know in what projects the assembly is.
Is better to have a new clean method on the compiler service, to ask if an assembly is a type provider and maybe return the name of the type providers contained in the assembly (to add child nodes on the reference to easier discover type providers?). A new issue anyway

dsyme · 2015-08-20T09:08:34Z

src/fsharp/fscopts.fs

@@ -444,7 +444,6 @@ let testFlag tcConfigB =
 // not shown in fsc.exe help, no warning on use, motiviation is for use from VS
 let vsSpecificFlags (tcConfigB: TcConfigBuilder) = 
  [ CompilerOption("vserrors", tagNone, OptionUnit (fun () -> tcConfigB.errorStyle <- ErrorStyle.VSErrors), None, None);
-    CompilerOption("validate-type-providers", tagNone, OptionUnit (fun () -> tcConfigB.validateTypeProviders <- true), None, None);


I do wonder if this flag should be left in and ignored.

In theory the next version of the Visual F# fsc.exe compiler will only be used with the corresponding Visual F# Tools and FSharp.Build.dll. But perhaps somehow the compiler will be invoked via the old tools, or an open source version of the tools, or some other tool that has copied out the command-line settings from the Visual F# Tools invocations of fsc.exe (which includes this flag). Perhaps just ignoring the option would be better.

👍 set as deprecated and noop. Because is a minor version (oob), no breaking changes
so if someone use it, get the warning but no error (build failed)
Next version (master?), cleanup and remove all deprecated commands?
I can do it, /cc @latkin

@dsyme yes I thought about that a bit, but dismissed it as it would need to be some kind of unsupported IDE setup... but I agree it's cheap and simple to keep it with a no-op so that we never have to worry. Anyone is welcome to send that in a PR.

msftclas added the cla-already-signed label May 13, 2015

enricosada changed the title ~~[WIPuse custom icon for assembly reference~~ [WIP] use custom icon for type provider assembly reference May 13, 2015

enricosada mentioned this pull request May 13, 2015

Remove type provider warning dialog #378

Closed

enricosada force-pushed the new_shiny_icon_for_type_provider_assembly_reference branch from e53ce24 to 678b2a3 Compare May 15, 2015 17:09

enricosada added 18 commits May 18, 2015 10:05

disable invalidation if tp not enabled

995501f

use custom type provider icon for assembly reference

bf4ca1b

the refresh is after build

removed TypeProviderSecurityDialog

08a943f

f typeprovidername is useful

70372bc

remove unused GetVerifiedPublisherInfo.GetVerifiedPublisherInfo

9bdd154

remove ApprovalIO.ReplaceApprovalStatus

2078831

removed ApprovalIO.ReadApprovalsFile

0841ebc

removed ApprovalIO.DoWithApprovalsFile

682f225

removed ApprovalIO.AppendApprovalStatus

78ccab6

remove TryDoWithFileStreamUnderExclusiveLockWithRetryFor500ms

234c9e9

removed ApprovalIO.TryDoWithFileStreamUnderExclusiveLock

8f3090f

remove approvals

fde4177

remove approvals

fe46509

removed ApprovalIO.ApprovalsAbsoluteFileName

59eeb24

removed ApprovalIO.partiallyCanonicalizeFileName, ApprovalIO.verifyIs…

2ac36e8

…PartiallyCanonicalized

remove tests helper ApproveAllMockTypeProviders

800f1b1

remove tests helper AddTypeProviderApprovedForDevelopment

a484cc7

remove tests helper ClearAllTypeProviderApprovals

4e33f80

enricosada added 18 commits May 21, 2015 18:10

inline always None parameter

3708022

inline always None parameter

0fac5ed

inline always None parameter

9f05976

inline always None parameter

cbd97f4

inline None

9de44b8

remove the global dialog

9ab57ed

remove useless GlobalsTheLanguageServiceCanPoke.theMostRecentFileName…

6b07c02

…WeChecked this global variable was used to save the last parsed file, to find current project. That doesn't work because if the file is a .fsx script, a standalone project is used instead of .fsproj

remove usage of GlobalsTheLanguageServiceCanPoke.displayLSTypeProvide…

e4e0b43

…rSecurityDialogBlockingUI

inline always None

8fba3ee

empty for

1893748

remove unused (i added this)

981f9eb

remove unused directive TYPE_PROVIDER_SECURITY

b45c580

remove unused method TypeProviderToolsOptionsPage.FindVisualChildByName

ca48b43

removed TypeProviderToolsOptionsPage

9e5d919

update comments

9e17c5a

cleanup

009e93e

renamed parameter

7db505c

update comments

a97dca6

fix build

5931058

latkin changed the title ~~use custom icon for type provider assembly reference~~ Remove type provider security dialog and use custom icon for type provider assembly reference Aug 4, 2015

latkin removed the V.Next label Aug 4, 2015

latkin added this to the F# 4.0 Update 1 milestone Aug 4, 2015

latkin closed this in 0f514ef Aug 19, 2015

latkin added the fixed label Aug 19, 2015

dsyme reviewed Aug 20, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove type provider security dialog and use custom icon for type provider assembly reference #448

Remove type provider security dialog and use custom icon for type provider assembly reference #448

enricosada commented May 13, 2015

allykzam commented May 13, 2015

forki commented May 13, 2015

enricosada commented May 13, 2015

enricosada commented May 13, 2015

KevinRansom commented May 13, 2015

latkin commented May 13, 2015

forki commented May 13, 2015

enricosada commented May 15, 2015

enricosada commented May 21, 2015

latkin commented Aug 17, 2015

latkin commented Aug 19, 2015

latkin commented Aug 19, 2015

enricosada commented Aug 20, 2015

dsyme Aug 20, 2015

enricosada Aug 20, 2015

latkin Aug 20, 2015

Remove type provider security dialog and use custom icon for type provider assembly reference #448

Remove type provider security dialog and use custom icon for type provider assembly reference #448

Conversation

enricosada commented May 13, 2015

allykzam commented May 13, 2015

forki commented May 13, 2015

enricosada commented May 13, 2015

enricosada commented May 13, 2015

KevinRansom commented May 13, 2015

latkin commented May 13, 2015

forki commented May 13, 2015

enricosada commented May 15, 2015

enricosada commented May 21, 2015

tools option page

update assembly icon strategy

latkin commented Aug 17, 2015

latkin commented Aug 19, 2015

latkin commented Aug 19, 2015

enricosada commented Aug 20, 2015

dsyme Aug 20, 2015

Choose a reason for hiding this comment

enricosada Aug 20, 2015

Choose a reason for hiding this comment

latkin Aug 20, 2015

Choose a reason for hiding this comment