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

Provide a built-in CLI handler for startup arguments (--squirel-*) #44

Closed
christianrondeau opened this issue Sep 21, 2014 · 7 comments
Closed

Comments

@christianrondeau
Copy link
Contributor

Since most apps will want/need to handle the --squirrel-* startup arguments, I suggest you provide a built-in handler for that. e.g.:

var myHandler = new MyImplementationOfISquirrelStartupArgumentsHandler();
if(SquirrelStartupArgumentsHandler.Handle(myHandler) == SquirrelStartupResult.Exit)
    return;

(Just as an example of the idea).

What do you think? I can provide my implementation back as a pull request if you want.

@christianrondeau christianrondeau changed the title Provide a built-handle handler for startup arguments (--squirel-*) Provide a built-in CLI handler for startup arguments (--squirel-*) Sep 21, 2014
@anaisbetts anaisbetts added this to the 1.0 - Debut to the world milestone Sep 21, 2014
@anaisbetts
Copy link
Contributor

Maybe? Send a PR :)

@christianrondeau
Copy link
Contributor Author

I can do that :) The only thing that still bothers me is where to put that code. I'm still uncomfortable with putting everything in UpdateManager. It's lifetime (and .Dispose() expected call) differs from a "runtime" API such as this one (and #47). Therefore, I'd create a SquirrelApplicationContext. What do you think?

@anaisbetts
Copy link
Contributor

I would put this in a separate class, yeah - it's not a context though, just make a simple static class + method that will parse arguments and return an Enum

@christianrondeau
Copy link
Contributor Author

Well, I thought of something like this, since first-run and install both require startup arguments:

var squirrelContext = new SquirrelApplicationContext(args);

if(squirrel.HandleInstall(handler));

if(squirrelContext.IsFirstRun)
  ShowSomeMessage();

So if we're to have such a class, why not make it so that it also handles creating the data directories (since that's the typical job of --squirrel-install), and if it knows the data directories, then it should be the main point of access.

But my main reason is that I'm a TDD-maniac, and if I'm to introduce a dependency in my app, I'd like to be able to mock it. Static methods just make my life harder :)

Anyway, I can just send a PR and let you decide what to do with it :)

@anaisbetts
Copy link
Contributor

So if we're to have such a class, why not make it so that it also handles creating the data directories

I'm not super excited about that, let's leave that in UpdateManager.

But my main reason is that I'm a TDD-maniac, and if I'm to introduce a dependency in my app, I'd like to be able to mock it

This method has no state though, what is there to mock? You pass in arguments, it does a thing

@christianrondeau
Copy link
Contributor Author

It's your project :) As for TDD, I'll end up wrapping it in a class anyway since I don't want to simulate Squirrel's logic in my app, I just want to simulate a state. My test would then look something like:

squirrelContextMock.SetupGet(m => m.IsFirstRun).Returns(true);

rather than passing the actual squirrel arguments. But that's just my own way of working, I won't push it on you!

@anaisbetts
Copy link
Contributor

#72 should cover this

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

No branches or pull requests

2 participants