-
Notifications
You must be signed in to change notification settings - Fork 268
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
Enable passing a percentage as a value for --cores #3357
Enable passing a percentage as a value for --cores #3357
Conversation
@@ -38,6 +38,12 @@ public class LitCommandWithRedirection : ILitCommand { | |||
errorFile = config.ApplySubstitutions(argumentsList[redirectErrorIndex + 1]).Single(); | |||
argumentsList.RemoveRange(redirectErrorIndex, 2); | |||
} | |||
var redirectErrorAppendIndex = argumentsList.IndexOf("2>>"); | |||
if (redirectErrorAppendIndex >= 0) { | |||
errorFile = config.ApplySubstitutions(argumentsList[redirectErrorAppendIndex + 1]).Single(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this code part, can you please explain it to me? I get that you want to redirect the standard error to the file as well, but I'm surprised that you have to introduce a new rule for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get that you want to redirect the standard error to the file as well, but I'm surprised that you have to introduce a new rule for that.
How would it know what 2>>
means without a new rule? It's a from scratch parser for lit commands so it doesn't understand anything by default. Maybe you could make it smart so if it understand 2
and >>
separately it can also combine them, but it can't do that right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would it know what 2>> means without a new rule? I
I mean, I'm surprised we hadn't had the need tor redirect error output before this PR, that's what I meant.
My comprehension problem was simpler. I just looked at the code above to see what it was doing, and I understand you are generalizing "2>", which I remembered was used in test cases. So that's good for me.
Co-authored-by: Mikaël Mayer <MikaelMayer@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a few tests to fix, but the functionality is working.
Fixes dafny-lang#3328 ``` --cores <count> Run the Dafny CLI using <n> cores, or using <XX%> of the machine's logical cores. [default: 6] ``` Default is 50% of logical cores. Sadly the help shows that number already being resolved. Changing that is complicated. <small>By submitting this pull request, I confirm that my contribution is made under the terms of the [MIT license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
Fixes dafny-lang#3328 ``` --cores <count> Run the Dafny CLI using <n> cores, or using <XX%> of the machine's logical cores. [default: 6] ``` Default is 50% of logical cores. Sadly the help shows that number already being resolved. Changing that is complicated. <small>By submitting this pull request, I confirm that my contribution is made under the terms of the [MIT license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
Fixes #3328
Default is 50% of logical cores. Sadly the help shows that number already being resolved. Changing that is complicated.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.