Skip to content
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

Fix off-by-one in log output line length #6896

Merged
merged 2 commits into from Jan 9, 2018
Merged

Fix off-by-one in log output line length #6896

merged 2 commits into from Jan 9, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jan 7, 2018

When a log line is over 255 characters, only the first 255 were printed, and the next line would start in the 257th, hence skipping one.

I'd rather get rid of the local, though, which I find distracting and prone to bugs like this one.

When a log line is over 255 characters, only the first 255 were printed, and the next line would start in the 257th, hence skipping one.
@SmallJoker
Copy link
Member

@pgimeno Removing the local variable is a good idea, as it will make the code shorter and easier to understand. The bugfix alone looks good.

@SmallJoker SmallJoker added the Bugfix 🐛 PRs that fix a bug label Jan 8, 2018
@rubenwardy
Copy link
Member

I'd prefer the local variable to removed, as it's pointless

@SmallJoker SmallJoker merged commit f77f19a into minetest:master Jan 9, 2018
@ghost ghost deleted the log-off-by-one branch January 9, 2018 18:24
t0ny2 pushed a commit to t0ny2/minetest that referenced this pull request Jan 23, 2018
sfan5 pushed a commit that referenced this pull request Feb 3, 2018
@sfan5 sfan5 mentioned this pull request Feb 3, 2018
sfan5 pushed a commit that referenced this pull request Apr 21, 2018
sfan5 pushed a commit that referenced this pull request May 13, 2018
osjc pushed a commit to osjc/minetest that referenced this pull request Jan 11, 2019
osjc pushed a commit to osjc/minetest that referenced this pull request Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants