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

Add support for launching the application with a file on Windows/Linux #647

Open
schaveyt opened this issue Dec 27, 2021 · 21 comments
Open
Labels

Comments

@schaveyt
Copy link

schaveyt commented Dec 27, 2021

The Problem

Applications built to edit file types typically prepare their electron.manifest.json files to perform file associations such that when a user double-clicks a custom filetype, the OS launches their applications.

Describe the solution you'd like

For both Windows and Linux, the OS will invoke the application executable and will pass it the location of the file to be processed as the first argument.

c:\path\to\my-custom-app\myapp.exe $1 

Per the Electron documentation for open-file, handling this must be done via the processing the process.argv array.

Now, there does not seem to be a defacto, standard way of passing this information around like there is for the commandline.hasSwitch(switch) and commandline.getSwitchValue(switch)..which is already supported by ElectronNET.

Therefore, I propose we treat the first argument as a special case by internally adding it to the switches as --open-file=$1 and then can be accessed using typical commandline.hasSwitch processing.

Example

  1. Invoking from electronize start

    c:\path\to\custom\app>electronize start /args .\file.custom --dog=woof --test=true
  2. Invoking the compiled Electron executable

    c:\path\to\custom\app>myapp.exe .\file.custom --dog=woof --test=true
  3. To access the commandline arguments

    // ASP.NET code...
    //
    await Electron.App.CommandLine.GetSwitchValueAsync("open-file"));  // returns ".\file.custom"
    await Electron.App.CommandLine.GetSwitchValueAsync("dog"));   // returns "woof"
    await Electron.App.CommandLine.GetSwitchValueAsync("test"));  // returns "true"

Additional Context

In my investigation in to implementing this on a fork, printing the process.argv[] array in ElectronNET.Host/main.js, the following behavior is observed.

  1. When running via electronize start /watch /args c:\path\to\custom\file.custom --dog=woof --test=true results in:

    process.argv[0]='C:\path\to\dotnet\project\obj\Host\node_modules\electron\dist\electron.exe'
    process.argv[1]='..\..\main.js'
    process.argv[2]='c:\path\to\custom\file.custom'
    process.argv[3]='--dog=woof'
    process.argv[4]='--test=true'
    process.argv[5]='--watch=true'
  2. When running via published executable found in the unpacked directory: C:\path\to\dotnet\project\bin\Desktop\win-unpacked\app.exe c:\path\to\custom\file.custom --dog=woof --test=true results in:

    process.argv[0]='C:\path\to\dotnet\project\bin\Desktop\win-unpacked\app.exe'
    process.argv[1]='c:\path\to\custom\file.custom'
    process.argv[2]='--dog=woof'
    process.argv[3]='--test=true'
@schaveyt
Copy link
Author

FYI - I have forked the repo and investigating an implementation

@danatcofo
Copy link
Contributor

danatcofo commented Dec 29, 2021

So this is generally accomplished by registering the App as a handler for the file type. The process to do this is documented in electron itself. Them when the app starts it will add the file to the start up args and you have to handle that case. If a file is passed again to the app via this method AND single instance is enabled it will fire off an event that you have to register a listener for. This again is all documented in electron.

What your after is already supported by electron though the implementation is slightly different between Mac and windows/linux. This is not something that we need to have a new feature for. I do this myself as is with my application already using the currently available functionality. I'd give you the code examples but I'm on vacation at the moment in a low data area.

Look up how electron does this and extrapolate the electron.net implementation. Should be straightforward to my recollection. If you can figure it out by Monday I'll follow up with some snippets that should work then.

Edit: so remember I said that I was in a low data area. I wrote that whole spiel above saying to go look it up in electron documentation before I saw that you updated the original with all the info you found looking up the electron documentation. My shoes taste great! However I still maintain that this is not content we need to build into electron.net itself. Could I make a suggestion that you create a nugget package that has an electron.net dependency that implements your method for accomplishing this? It's an opinionated direction your going with whatever you develop. I have an opinionated implementation as well.

@schaveyt
Copy link
Author

schaveyt commented Dec 29, 2021

@danatcofo I just saw your comment as I was creating the PR 😃 . take a look at the change a chew on it a bit. Having this implemented will make for parity of behavior between the various operating systems and well...continue to make ElectronNET more appealing that MAUI.

Updated: I am not beholden to my implementation, just that I can have this capability 👍

@schaveyt
Copy link
Author

schaveyt commented Dec 29, 2021

I have the file associations already working via the manifest. When my users double click the file, it correctly launches the application...but I have no way to determine what file was used so it sits at the initial screen rather than opening the file.

In this invocation path, ElectronNET ignores that first argument (which is the path to the file) as it is setup for the /args style. If you have a way of already doing this for win and linux I would be very interested in any pro tips.

@dlitty
Copy link

dlitty commented Dec 29, 2021

FYI, I would have to dig back through the details, but the application that made me open #478 worked fine on Windows as well as OSX. If I remember correctly, the full path of the file was sent in via a command line parameter that I could then open. I can look up the implementation in my app if that would help.

@schaveyt
Copy link
Author

schaveyt commented Dec 29, 2021

@dlitty that would be most helpful. 👍

@dlitty
Copy link

dlitty commented Dec 31, 2021

In my case, I basically did what you are proposing, with the exception that even the file parameter itself was a named parameter:

    var file = await Electron.App.CommandLine.GetSwitchValueAsync("file");
    // ... process the value of file as needed

Then, as part of the build script, I added the correct registry entry to be passed as a parameter:

   reg add HKCR\MYEXTENSION\shell\open\command /ve /d "c:\lsi\MyApp\MyApp.exe --file=\"%%1\""

I realize this has the downside of having to build it on each machine where you want to run it - which in my case was fine, since it is only used by developers. I don't know without digging through the docs if that can be automated as part of the packaging process? If not, your application could potentially check for the registry entry being there and add it if not (so the first run would not obey the parameter, but subsequent runs would).

Hope this helps!

@schaveyt
Copy link
Author

Thanks for looking into that @dlitty. Hmm, this was a solution I was aware of and had investigated leading up to my proposed change. That approach, while technically possible, seemed an out-of-the-way workaround for what the electronn doc say to use. (i.e. gaining access and parsing the process argv array.) The argv keeps is all simple and consistent to use for both windows and Linux deployments.

Unfortunately, this array is not correctly accessible in the ElectronNET codebase and the closest thing was the previous work done for the CommandLine interface. I totally get my inital update in the PR is a bit jenky and perhaps a better solution is to and a "Process" interface instance to the Electron instance with than argv populated.

@dlitty your thoughts either way?

@danatcofo
Copy link
Contributor

I think ensuring the electron args are properly available to the dotnet runtime if they aren't already would be the appropriate change. This would mean that the aspnet app could take appropriate action on startup and not have the interpretation of the args be injected by electron.net inappropriately.

@dlitty
Copy link

dlitty commented Jan 1, 2022

@schaveyt I agree. The only reason it worked for me, like I said, was because this particular app is never distributed - it’s always just built and run. In addition, this particular app was only needed on Mac and Windows, not Linux. Obviously having a scalable solution that works across all platforms is desired; I think the closest we can come to that is making the “actual” argv array accessible to the .NET process. MacOS is always going to be a special case due to the way the OS handles file associations, but windows and Linux could be consistent by exposing the argv array.

I don’t really like the idea of treating the first argument as special, since there may very well be cases where that would not be desirable. So while it adds a little bit more work for the developer, I think the cleanest solution is exposing the argv array directly.

And, btw, I have another app in the works that could benefit from that - so I’m listening to this thread with great interest :-)

@schaveyt
Copy link
Author

schaveyt commented Jan 1, 2022

@danatcofo and @dlitty I think ther is consensus on the strategy, now onto the tactical aspects. Let me spend a bit this afternoon investigating the codebase and offer a proposal to make the argv available to asp.net app.

@schaveyt
Copy link
Author

schaveyt commented Jan 1, 2022

Option 1 (Preferred)- Pass the process.argv elements to the AspCoreBackend process

This option simply give the AspCoreBackend access to the same args passed to the electron process allowing standard c# handling of commandline args 👍

Today, the AspCoreBackend is started in two different flavors "Normal" and "WithWatch" but receive a common set of args as described below:

Normal:

This mode is used when called using the electronize start or when the compiled/published Electron app is deployed. The backend executable and args invoked as follows:

${aspcore-executable} --environment=Development /electronPort=8000 /electronWebPort=8001

WithWatch:

This mode is used when called using the electronize start /watch exclusively during development. The backend executable and args invoked as follows:

dotnet watch run --environment=Development /electronPort=8000 /electronWebPort=8001

The Change

Prepend the current AspCoreBacked args with the args passed to the electron process.

Example: my-electron-program /some/directorey/custom-file.xyz --some-option /test=true

# Normal:
${aspcore-executable} /some/directorey/custom-file.xyz --some-option /test=true --environment=Development /electronPort=8000 /electronWebPort=8001
# WithWatch
dotnet watch run /some/directorey/custom-file.xyz --some-option /test=true --environment=Development /electronPort=8000 /electronWebPort=8001

NOTE: per the dotnet CLI, the passing of application arguments should really be preceded with the
chars -- as described here.
Thus making the withWatch correctly look like this:

# withWatch
dotnet watch run -- /some/directorey/custom-file.xyz --some-option /test=true --environment=Development /electronPort=8000 /electronWebPort=8001

Option 2 - Add the Process class to the ElectronNET.API

This may overkill. In the same way the CommandLine was added, one could add a new Process class and expose the argv property. This would touch quite a few more areas of the codebase. I am not sure what this fully entails but something like:

  1. Create a ElectronNET.Host/api/process.ts to interact with Electron's process object from the node side
  2. Create a ElectronNET.API/Process.cs to bridge with the electron api
  3. Update ElectronNET.Host/main.js to initialize the processApi object.

Let me know your thoughts and I will be glad to volunteer either implementation.

@danatcofo
Copy link
Contributor

I prefer the process class option. I think this keeps things cleaner overall and provides more options to consumers over the long run. It also becomes an elective query va trying to fenangle a bunch of things into the startup parameters that may cause confusion.

@schaveyt
Copy link
Author

schaveyt commented Jan 1, 2022

@danatcofo sounds good. For those that might be following and have a similar questio. With Option 2, where in the asp.net code would you recommend placing code to talk to the ElectonNET API if one is trying to access the args as early as possible?

@danatcofo
Copy link
Contributor

Well there are 2 ways we can do this.

  1. Setup a process.js class that we can query like we do app or any of the others or set environment variables on
  2. Set environment variables on the asp net process that is spun up in main.js

You could do both but that seems redundant. Consistency would say doing the process.js method but that forces the asp.net app to wait for the socket to connect. That may or may not be desired, don't know,

I think I would prefer the process.js method more as it's much more exposed and discoverable.

@schaveyt
Copy link
Author

schaveyt commented Jan 2, 2022

Fair enough. It's easy to start with the process.js which will likely hit 80% of the use cases...then address the 20% if and when they come along 👍.

As I'm house bound due to the current snow storm, I'll interleave a PR update with new episodes of The Witcher 🤘

schaveyt pushed a commit to schaveyt/Electron.NET that referenced this issue Jan 2, 2022
schaveyt pushed a commit to schaveyt/Electron.NET that referenced this issue Jan 2, 2022
schaveyt pushed a commit to schaveyt/Electron.NET that referenced this issue Jan 3, 2022
schaveyt pushed a commit to schaveyt/Electron.NET that referenced this issue Jan 3, 2022
@schaveyt
Copy link
Author

schaveyt commented Jan 3, 2022

PR #648 is now updated per the above comments. I have included a summary of the changes, how I verified it, and the results.

@danatcofo if and when this change is approced and merge, what are the current plans for rolling our a updated nuget version with this and any other pending PRs?

schaveyt pushed a commit to schaveyt/Electron.NET that referenced this issue Jan 3, 2022
@danatcofo
Copy link
Contributor

It's in @GregorBiswanger hands for merging and such, I'm simply an engaged party having a public app based on this project. :) he mentioned elsewhere that he has some vacation that is in effect now that will give him time to do the updates. I'd imagine it will happen this week or next. /shrug

schaveyt pushed a commit to schaveyt/Electron.NET that referenced this issue Jan 4, 2022
…ly access to props

This is to address a PR ElectronNET#648 review comment to ensure that only the external
users are not able to modify the instance values.
@camwar11
Copy link

It looks like #648 was merged at some point, but then the functionality was removed in d3aa978. Was this intentional?

@FlorianRappl
Copy link
Collaborator

I think it was not - if we could get that back it would be awesome!

@dlitty
Copy link

dlitty commented Jul 19, 2023 via email

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

No branches or pull requests

5 participants