Skip to content

Conversation

@jkotalik
Copy link
Contributor

When writing to the stdout file with ANCM, for some reason, \r\n get interpreted as two new lines when writing to an ostream. This change replaces \r\n with \n before writing to the file.

@jkotalik jkotalik requested a review from BrennanConroy August 17, 2020 22:50
@ghost ghost added the area-servers label Aug 17, 2020
@Tratcher
Copy link
Member

Why? Is there an issue for this?

@jkotalik
Copy link
Contributor Author

@BrennanConroy do you remember if there was an issue for this? IIRC there was one filed in the past.

We have noticed it multiple times when debugging stdout logs from customers and other things.

@BrennanConroy
Copy link
Member

I know I complained about it in the past, there may have been an issue at one point from an external customer...

const auto multiByte = to_multi_byte_string(text, CP_UTF8);
auto multiByte = to_multi_byte_string(text, CP_UTF8);

// Writing \r\n to an ostream will cause two new lines to be written rather
Copy link
Member

Choose a reason for hiding this comment

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

Will \n will be written without \r in the final file output?

Also, I don't think ostream was the issue, iirc it was a Windows API we were calling the duplicated the newline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, when I was testing it, I called ostream << "test\r\n" and I see two new lines in the file.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this ostream the fake file that is then redirected? I could be wrong, it has been a long time since I looked at this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe so, the m_file points directly to the stdout file.


Assert.Contains("TEST MESSAGE", contents);
Assert.DoesNotContain("\r\n\r\n", contents);
Assert.Contains("\r\n", contents);
Copy link
Member

Choose a reason for hiding this comment

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

Thanks

@jkotalik jkotalik added the tell-mode Indicates a PR which is being merged during tell-mode label Aug 18, 2020
@jkotalik jkotalik merged commit cdb3cbd into release/5.0 Aug 18, 2020
@jkotalik jkotalik deleted the jkotalik/fixDoubleSpace branch August 18, 2020 16:34
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions tell-mode Indicates a PR which is being merged during tell-mode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants