Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

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

Closed
lodejard opened this issue Mar 13, 2015 · 21 comments · Fixed by #2263 or #2269
Closed

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

lodejard opened this issue Mar 13, 2015 · 21 comments · Fixed by #2263 or #2269

Comments

@lodejard
Copy link
Contributor

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
Copy link
Member

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

@Mpdreamz
Copy link

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

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

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

Possible "Host Options" include:

  • --debug - Wait for a debugger to attach
  • --lib <path> - Add a search path for assemblies
  • --appbase|-p <path> - Change the base path for the application (defaults to .)

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 ☀️ ✨ 😄.

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
Copy link
Member

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

@DamianEdwards
Copy link
Member

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

@lodejard
Copy link
Contributor Author

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

@shanselman
Copy link

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 [email protected]
wrote:

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


Reply to this email directly or view it on GitHub
#1403 (comment).

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

@muratg muratg added this to the 1.0.0-beta6 milestone Jul 6, 2015
@muratg
Copy link
Contributor

muratg commented Jul 6, 2015

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
Copy link
Contributor

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
Copy link
Member

@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
Copy link
Contributor

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

@shanselman
Copy link

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 [email protected]
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
#1403 (comment).

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

@davidfowl
Copy link
Member

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
Copy link

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 [email protected]
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
#1403 (comment).

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

@moozzyk
Copy link
Contributor

moozzyk commented Jul 15, 2015

@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
Copy link

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

On Wed, Jul 15, 2015 at 11:11 AM, Pawel Kadluczka [email protected]
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
#1403 (comment).

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

@analogrelay
Copy link
Contributor

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
Copy link
Contributor

Of course we should verify this :)

@muratg
Copy link
Contributor

muratg commented Jul 15, 2015

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.

@muratg muratg modified the milestones: 1.0.0-beta7, 1.0.0-beta6 Jul 15, 2015
moozzyk pushed a commit that referenced this issue Jul 16, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 16, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 16, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 16, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 16, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 21, 2015
Before providing appbase folder was necessary. Now we don't expect a folder anymore but assume that the current folder is the appbase folder. The only exception is where for backwards compatibility we continue to recognize `.` (so `dnx . run` continues to work). If the user really wants to provide a folder that is not the current folder they need to use the --appbase parameter.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 21, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 22, 2015
Before providing appbase folder was necessary. Now we don't expect a folder anymore but assume that the current folder is the appbase folder. The only exception is where for backwards compatibility we continue to recognize `.` (so `dnx . run` continues to work). If the user really wants to provide a folder that is not the current folder they need to use the --appbase parameter.

Fixes #1403
moozzyk pushed a commit that referenced this issue Jul 22, 2015
The user no longer has to provide folder (or `.`) when running dnx commands.

Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.

Fixes #1403
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.