-
-
Notifications
You must be signed in to change notification settings - Fork 616
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
[core] new cmd path decorations with %[] syntax #769
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -281,3 +281,18 @@ | |
local version = os.getversion(); | ||
test.istrue(version ~= nil) | ||
end | ||
|
||
|
||
|
||
-- | ||
-- os.translateCommandsAndPaths. | ||
-- | ||
|
||
function suite.translateCommandsAndPaths() | ||
test.isequal('cmdtool "../foo/path1"', os.translateCommandsAndPaths("cmdtool %[path1]", '../foo', '.', 'osx')) | ||
end | ||
|
||
function suite.translateCommandsAndPaths_PreserveSlash() | ||
test.isequal('cmdtool "../foo/path1/"', os.translateCommandsAndPaths("cmdtool %[path1/]", '../foo', '.', 'osx')) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These tests are great, but I've noticed a bit of a trend of our Lua patterns not working with the same pattern twice. For example, what happens if you were to have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can certainly add a test for that... the output should be We use this in several places like that... but adding a test for it is probably a good idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome, I can never really tell with the Lua patterns. 👍 |
||
|
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 tend to agree with the code here, just wanted to know your thoughts on having the caller perform this? Would there be any benefit in this function being "flexible"?
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 think it matters much, whatever we prefer... it just moves this single line into 5-6 locations instead...