Feature: Allow inline statements and added tests for Windows #13
Conversation
Merge pull request FauconFan#10 from exsjabe/master
src/cmdrun.rs
Outdated
| let mut res = String::with_capacity(str.len()); | ||
| match inline { | ||
| true => { | ||
| let str = str.trim_end(); | ||
| let count = str.lines().count(); | ||
| for (i, line) in str.lines().enumerate() { | ||
| res.push_str(line); | ||
| if i < count - 1 { | ||
| res.push_str("\r\n"); | ||
| } | ||
| } | ||
|
|
||
| res | ||
| } | ||
| false => { | ||
| for line in str.lines() { | ||
| res.push_str(line); | ||
| res.push_str("\r\n"); | ||
| } | ||
|
|
||
| res | ||
| } | ||
| } |
There was a problem hiding this comment.
This behavior should be documented. I'm not sure what this does. Does that replace '\n' by '\r\n' when the generated output from scripts run by Windows Shell? Because these scripts does not separate lines using \r\n by default? Maybe it is my poor understanding of how Windows IO works...
There was a problem hiding this comment.
Maybe the inline match block can be replaced by a string join, then when inline is false, add an additional \r\n.
Something like:
let mut res = str.lines().iter().collect::Vec<_>::().join("\r\n");
if !inline {
res.push_str("\r\n");
}
If that's a performance choice, I can hear it, but everything is linear here, we don't really care about performance for this tool.
There was a problem hiding this comment.
Not sure myself if it's even necessary, I was working backwards from trying to get the regression tests to work.
Some commands like yes output unix-style linebreaks which don't start with\r and that caused the tests to break. In practice, a user wouldn't notice the difference, but I suppose there is value in having consistency? Either way your solution is way cleaner.
There was a problem hiding this comment.
Theoretically someone could be using a tool to verify the resulting markdown and not be able to easily validate it because of inconsistent line breaks, I suppose
There was a problem hiding this comment.
This is interesting ahah. Thank you for the fix!
|
This should fix the issues you pointed out in the comments 😁 |
Inline Statements
This solves issue #9 by running two regex's, one for statements containing a linefeed, and one without. Then passing down a feature flag based on if the statement is
inlineor not. I have added unit tests as well as regression tests for this feature.Windows Tests
This PR also solves the problems with running tests on Windows discussed in PR #10. The same solution can probably be expanded to solve issue #7 as well.