Skip to content
This repository was archived by the owner on Jun 3, 2021. It is now read-only.

make debugging output for protocol structs better readable #162

Merged
merged 7 commits into from
Mar 19, 2019

Conversation

soenkehahn
Copy link
Contributor

@soenkehahn soenkehahn commented Mar 19, 2019

Fixes #149.

@ghost ghost assigned soenkehahn Mar 19, 2019
@ghost ghost added the in progress label Mar 19, 2019
@soenkehahn soenkehahn force-pushed the sh/debug branch 4 times, most recently from 32ca8c4 to 21e034a Compare March 19, 2019 18:27
@soenkehahn
Copy link
Contributor Author

I didn't change Step.stdout and Protocol.stderr, because those strings could theoretically contain null bytes and those are not allowed in OsString or PathBuf.

@soenkehahn soenkehahn requested a review from hallettj March 19, 2019 18:30
@soenkehahn soenkehahn marked this pull request as ready for review March 19, 2019 18:30
@soenkehahn soenkehahn force-pushed the sh/debug branch 5 times, most recently from ac35f64 to b2f1789 Compare March 19, 2019 19:02
Copy link
Contributor

@hallettj hallettj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I think it is nice to be using Path and PathBuf in more places.

if self.protocol.mocked_files.contains(&filename) {
let statbuf_ptr = registers.rsi;
let mock_mode = if filename.ends_with(b"/") {
let mock_mode = if filename.as_os_str().as_bytes().ends_with(b"/") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a little checking, and learned that there is an RFC and tracking issue for an ends_with method for OsString. But it is not ready yet.

@soenkehahn soenkehahn merged commit dfcf107 into master Mar 19, 2019
@soenkehahn soenkehahn deleted the sh/debug branch March 19, 2019 21:51
@ghost ghost removed the in progress label Mar 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants