Skip to content

Comments

create .NET Interactive extension package#155

Merged
cartermp merged 11 commits intofslaborg:masterfrom
colombod:create_interactive_extension
Feb 8, 2021
Merged

create .NET Interactive extension package#155
cartermp merged 11 commits intofslaborg:masterfrom
colombod:create_interactive_extension

Conversation

@colombod
Copy link
Collaborator

@colombod colombod commented Feb 3, 2021

Add .NET Interactive extension pakcage

Copy link
Collaborator

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

To get CI to pass I think we'll also have to change the tool manifest file and pull in the very latest version of paket and fake-cli

Comment on lines 18 to 29
let newScript = StringBuilder()
newScript.AppendLine("""<script type="text/javascript">""") |> ignore
newScript.AppendLine("""
var renderPlotly = function() {
var xplotRequire = require.config({context:'xplot-3.0.1',paths:{plotly:'https://cdn.plot.ly/plotly-1.49.2.min'}}) || require;
xplotRequire(['plotly'], function(Plotly) {" """) |> ignore
newScript.AppendLine(script) |> ignore
newScript.AppendLine(@"});
};"
) |> ignore
newScript.AppendLine(JavascriptUtilities.GetCodeForEnsureRequireJs(Uri("https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js"), "renderPlotly")) |> ignore
newScript.ToString()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for fun, this can now be an interpolated string since it's using the .NET 5 SDK

NuGet.config Outdated
<add key="dotnet3-dev" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet3.1/nuget/v3/index.json" />
<add key="dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
<add key="MachineLearning" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/MachineLearning/nuget/v3/index.json" />
<add key="PSGallery" value="https://www.powershellgallery.com/api/v2/" />

Choose a reason for hiding this comment

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

Unless you are going to pull PowerShell modules in this repo, this feed probably should be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I have to implement in this repo the code that the powershell kernel was doing to integrate the xplot types. What do you think?

Choose a reason for hiding this comment

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

Yes, the New-PlotyChart cmdlet needs to be moved here. But the PSGallery is not needed for that. You will need to reference to PowerShellStandard.Library.

Choose a reason for hiding this comment

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

This entry should be removed.

add extension method


update tools


updated build


update Fsharp.Formatting.CommandTool


update test project


update tfm


installed updates


move to right folder


add powershell kernel logic


wip
Comment on lines 21 to 23
override _.BeginProcessing() = ()

override _.ProcessRecord() = ()

Choose a reason for hiding this comment

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

Please do the same as the original implementation. This makes sure the piping behavior is as expected.

        private List<Trace> _traces;

        /// <summary>
        /// BeginProcessing override.
        /// </summary>
        protected override void BeginProcessing()
        {
            _traces = new List<Trace>();
        }
        
        /// <summary>
        /// ProcessRecord override.
        /// </summary>
        protected override void ProcessRecord()
        {
            _traces.AddRange(Trace);
        }

NuGet.config Outdated
<add key="dotnet3-dev" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet3.1/nuget/v3/index.json" />
<add key="dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
<add key="MachineLearning" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/MachineLearning/nuget/v3/index.json" />
<add key="PSGallery" value="https://www.powershellgallery.com/api/v2/" />

Choose a reason for hiding this comment

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

This entry should be removed.



override this.EndProcessing() =
let traces = if isNull this.Trace then [||] else this.Trace

Choose a reason for hiding this comment

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

This line can be removed now

@colombod colombod requested a review from cartermp February 8, 2021 19:55
@colombod colombod marked this pull request as ready for review February 8, 2021 20:18
@cartermp cartermp merged commit aafde05 into fslaborg:master Feb 8, 2021
@colombod colombod deleted the create_interactive_extension branch February 9, 2021 01:46
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.

4 participants