catchorg / Clara

A simple to use, composable, command line parser for C++ 11 and beyond

Boost Software License 1.0

648 stars 67 forks source link

Clara: TextFlow: fixed issue with word wrapping #74

Closed JoeyGrajciar closed 5 years ago

JoeyGrajciar commented 5 years ago

The issue with word wrapping was that in case that m_pos + width was greater or equal to m_end it fallbacked to m_end-m_pos branch which caused that printed string was greater than configured width.

Also caused duplication of words as in next iteration m_len was added to m_pos this was less then m_end.

Originally reported in https://github.com/catchorg/Catch2/issues/1400

codecov[bot] commented 5 years ago

Codecov Report

Merging #74 into master will increase coverage by 0.12%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   94.72%   94.85%   +0.12%     
==========================================
  Files           2        2              
  Lines         683      680       -3     
==========================================
- Hits          647      645       -2     
+ Misses         36       35       -1

Impacted Files	Coverage Δ
include/clara_textflow.hpp	`98.93% <100%> (+0.5%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a19db09...a1b6fd6. Read the comment docs.

horenmar commented 5 years ago

Thanks, but you should also add tests that show the bug and protect against regressions.

JoeyGrajciar commented 5 years ago

Yes i thought about them. But I was not sure if I should add them here or to textflowcpp repository :) So i will probably add tests here and also copy existing ones from the textflowcpp repo.

JoeyGrajciar commented 5 years ago

But for tests copied from textflowcpp repository the iterator traits pull request would need to be merged :)

horenmar commented 5 years ago

Oh, right.

I'll think about the best solution to this, maybe I should pester Phil to move textflow to catchorg as well.

JoeyGrajciar commented 5 years ago

In that case my pull requests should be done against the textflowcpp repo. either way compilation of its tests is failing without iterator_traits pull request :)

philsquared commented 5 years ago

Thanks @JoeyGrajciar. I've applied this fix, along with a test based on your associated issue, to the TextFlowCpp project:

https://github.com/philsquared/textflowcpp/commit/1538b0cea716f9fabd85974fe654a501afc8c1a7

I'll get that merged into Clara soon.

JoeyGrajciar commented 5 years ago

@philsquared thank you :) Closing this pull request.