issue1158-stdin-for-single-files #1516
Conversation
|
Thanks for your pull request and interest in making D better, @andre2007! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. |
source/dub/internal/utils.d
Outdated
| @@ -40,8 +40,14 @@ NativePath getTempDir() | |||
| NativePath getTempFile(string prefix, string extension = null) | |||
| { | |||
| import std.uuid : randomUUID; | |||
| import std.array: replace; | |||
There was a problem hiding this comment.
I think the existing file uses tabs whereas you use whitespace here. We should probably just convert all files to whitespace at some point...
| @@ -0,0 +1,7 @@ | |||
| /+ dub.sdl: | |||
| name "hello" | |||
There was a problem hiding this comment.
The name is optional since 1.6
There was a problem hiding this comment.
By removing name attribute, the file not longer compiles. I tried different formats:
/+ dub.sdl:
+/
/+ dub.sdl
+/
There was a problem hiding this comment.
Ah sorry. Apparently one items is needed then :/
| string fileName = prefix ~ "-" ~ randomUUID.toString() ~ extension; | ||
|
|
||
| if (extension !is null && extension == ".d") | ||
| fileName = fileName.replace("-", "_"); |
There was a problem hiding this comment.
randomUUID also returns a string containing dashes. Therefore my idea was to keep the dash and replaces all dashes in case of a d module file.
|
Maybe a short changelog entry so that people know about this? |
|
Change log added |
source/dub/commandline.d
Outdated
| import dub.internal.utils: getTempFile; | ||
|
|
||
| auto path = getTempFile("app", ".d"); | ||
| std.file.write(path.toNativeString(), stdin.byLineCopy.join); |
There was a problem hiding this comment.
FYI if you don't want the the FQSN (std.file), there exists toFile just for this reason:
stdin.byChunk(4096).joiner.toFile(path.toNativeString())
Also use byChunk instead of byLineCopy to avoid unnecessary allocation + copying and as a bonus you don't loose the line endings.
Furthermore with joiner you avoid allocations :)
There was a problem hiding this comment.
great! looks much better now.
There was a problem hiding this comment.
DMD 2.071.2 and lower does not support toFile ):
source/dub/commandline.d
Outdated
|
|
||
| auto path = getTempFile("app", ".d"); | ||
| std.file.write(path.toNativeString(), stdin.byLineCopy.join); | ||
| args = args[0]~[path.toNativeString()]~args[2..$]; |
There was a problem hiding this comment.
No need for [] here.
Also mind the style of the existing file (and DStyle): in other words spaces around ~.
There was a problem hiding this comment.
Square brackets are needed.
Args without []: ["C:\Users\home\git\dub\bin\dub.exeC:\Users\home\Desktop\abc.d"]
Args with []: ["C:\Users\home\git\dub\bin\dub.exe", "C:\Users\home\Desktop\abc.d"]
|
@wilzbach Unfortunately I had to switch back to std.file.write as toFile isn't available in the lower DMD/Phobos releases. The write function also had some issue with the joiner result, I had to add .array. Will we stay on 2.068.2 as lowest version to be supported or is there a plan to remove some of the oldest versions? |
|
The latest GDC release has been the blocking reason for an upgrade. I can make that upgrade tomorrow. |
|
@wilzbach did you already had the chance to make the upgrade (GDC was blocking)? |
|
Not yet, but done now #1520 (might require a few tries of experimentation to get it right tough, was a bit more complex than I remembered). So if you don't want to depend/wait on this, feel free to revert here for now ;-) |
|
Yes I really like your proposed syntax (toFile). I will revert to your proposal when the merge was done. |
|
It seems there is a general error in semaphoreci. The gdc build fails with: Also the dmd build fails, here no clear error description can be found. https://semaphoreci.com/wilzbach/dub/branches/pull-request-1516/builds/7 |
|
Yes, we need to merge #1520 first (I changed the scripts while I was setting up the newer |
|
@wilzbach I changed the coding to your proposal (stdin.byChunk(4096).joiner.toFile(path.toNativeString());) and it works fine. Except some dub packages throwing errors. But I assume this is a general issue. |
This pull request allows running single source code files from stdin.
Example:
curl -s http://localhost:8080/singledub.d | dub -
The - (dash) is the marker for reading stdin.