aspnet / dnx

OBSOLETE - see readme
Other
963 stars 224 forks source link

dnx requiring "run" can be improved a bit #1403

Closed lodejard closed 9 years ago

lodejard commented 9 years ago

dnx path-to-project.json run is a bit redundant - if you name the project file or folder, and there are no arguments, it can be implied.

On the other hand, if dnx run can use the current directory as the project then that does make sense, and looks cleaner than dnx . run or dnx project.json run

davidfowl commented 9 years ago

And if there's a folder called run. We can just ignore it? What about the other commands?

Mpdreamz commented 9 years ago

I would love if we could change it to dnx run [folder | file] with it defaulting to pwd. Feels more posixy this way.

If you have a folder named run it'll be dnx run run :)

shanselman commented 9 years ago

Ya, this all started with the whole "dnx ." thing and I came here to open another bug but found this.

When I went "dnx web" I get a lousy error "Please specify the command to run" and I just stared.

I agree with @Mpdreamz we should do "dnx command [directory]" so the default is .

@glennc @danroth27 @DamianEdwards

(or at the absolute minimum, change the error to tell folks about the . )

lodejard commented 9 years ago

Yeah, was talking with @anurse about this the other day too. Specifying the project dir would probably move to a switch, because things coming after the "command" should be treated as string[] args. And the main use-case would still be to default to the current working directory, so you'd almost never be exposed to the project dir switch.

analogrelay commented 9 years ago

My thought was that dnx.exe would have the following syntax:

dnx <Host Options> [Command] <App Args...>

Possible "Host Options" include:

The first argument that doesn't start with - indicates the immediate end to Host Options. Everything from the first non-- argument on is passed along to AppHost.

When DNX starts up, it uses the --appbase value (which defaults to .) as the base path and boots AppHost in that directory, the rest of the arguments go to that. So if you are in a project.json directory, you can just dnx run and everything is :sunny: :sparkles: :smile:.

There is a small exception, which is that if [Command] resolves to a real physical file on disk (a DLL or EXE, for example), that file is treated as a complete application. This lets you do dnx MyApp.exe (similar to mono MyApp.exe).

If you want to run commands from another project, you just do dnx -p path/to/project run. It's a few extra characters than it is today (-p) but since that's the secondary case, I think that's OK.

davidfowl commented 9 years ago

It's worth noting that changing this might break visual studio /cc @sayedihashimi @PradeepKadubandi @BillHiebert

DamianEdwards commented 9 years ago

I really like the look of @anurse's proposed changes.

lodejard commented 9 years ago

+1 and agreed such a change would need to be coordinated pretty carefully

shanselman commented 9 years ago

I like this also. The 90% case has to be as easy as possible. "mono foo," "java foo, "dnx foo"

On Wed, May 20, 2015 at 1:08 PM, Louis DeJardin notifications@github.com wrote:

+1 and agreed such a change would need to be coordinated pretty carefully

— Reply to this email directly or view it on GitHub https://github.com/aspnet/dnx/issues/1403#issuecomment-104020295.

Scott Hanselman Donate to fight diabetes: http://hnsl.mn/fightdiabetes

muratg commented 9 years ago

We'll go with @anurse's suggestion. We need to ensure tooling doesn't break.

Also let's keep dnx . run behavior so that we don't break people who are super used to this. We'll remove this later.

BillHiebert commented 9 years ago

Tooling will need to be updated to handle the changes. We should be able to do it in a compatible way that continues to support the old syntax for older versions of DNX.

davidfowl commented 9 years ago

@BillHiebert I dont think it will affect tooling. Can you tell us how you're launching things today? I believe you're passing in the full path including the --appbase flag which should continue to work.

BillHiebert commented 9 years ago

I think you are right. We would continue to pass in the -appbase and --lib like we do today.

shanselman commented 9 years ago

Why do we need to support older versions? It hasn't been released and there is no go-live. This is the time to break things and fix this.

On Tue, Jul 7, 2015 at 10:31 AM, BillHiebert notifications@github.com wrote:

Tooling will need to be updated to handle the changes. We should be able to do it in a compatible way that continues to support the old syntax for older versions of DNX.

— Reply to this email directly or view it on GitHub https://github.com/aspnet/dnx/issues/1403#issuecomment-119276764.

Scott Hanselman Donate to fight diabetes: http://hnsl.mn/fightdiabetes

davidfowl commented 9 years ago

To avoid pissijg people off we stage changes like these. If a nightly build breaks your vs then we want to line it up with a VS release

shanselman commented 9 years ago

Sure, but we aren't even releasing yet. We have some time, no? I say break stuff. Not to mention we could have fixed this in May.

On Tue, Jul 7, 2015 at 11:36 AM, David Fowler notifications@github.com wrote:

To avoid pissijg people off we stage changes like these. If a nightly build breaks your vs then we want to line it up with a VS release

— Reply to this email directly or view it on GitHub https://github.com/aspnet/dnx/issues/1403#issuecomment-119295943.

Scott Hanselman Donate to fight diabetes: http://hnsl.mn/fightdiabetes

moozzyk commented 9 years ago

@davidfowl @shanselman @DamianEdwards @anurse @muratg - in my PR I kept the ability to specify a folder (including .) for backwards compatibility. If we don't want to keep this (or just special case .) this is the time to speak up (@anurse already did :)). Note that to keep this behavior we need to go to the disk to be able to disambiguate between a command and a folder unless it is a ..

shanselman commented 9 years ago

Since we can't break tooling, I think we need to keep it.

On Wed, Jul 15, 2015 at 11:11 AM, Pawel Kadluczka notifications@github.com wrote:

@davidfowl https://github.com/davidfowl @shanselman https://github.com/shanselman @DamianEdwards https://github.com/DamianEdwards @anurse https://github.com/anurse @muratg https://github.com/muratg - in my PR I kept the ability to specify a folder (including .) https://github.com/aspnet/dnx/pull/2263/files?diff=unified#diff-c6b566f7546118bf1810a8069541a006R151 for backwards compatibility. If we don't want to keep this (or just special case .) this is the time to speak up (@anurse https://github.com/anurse already did :)). Note that to keep this behavior we need to go to the disk to be able to disambiguate between a command and a folder unless it is a ..

— Reply to this email directly or view it on GitHub https://github.com/aspnet/dnx/issues/1403#issuecomment-121698700.

Scott Hanselman Donate to fight diabetes: http://hnsl.mn/fightdiabetes

analogrelay commented 9 years ago

We may be OK from the tooling perspective. Last I heard the tooling was always specifying --appbase which would continue to work even if we took the break-the-world approach. /cc @BillHiebert @PradeepKadubandi

analogrelay commented 9 years ago

Of course we should verify this :)

muratg commented 9 years ago

Based on a talk with @davidfowl early this morning, postponing this to beta 7. We'll also start integrating with tooling on a much higher frequency soon so we'll find any breaking issues ASAP. It's just late to check this in beta 6.