adds ability to break lines when rebasing

[tomjemmett]

• [x] added unit tests and checked code coverage with covr::report() (should aim for 100%)
• [x] ran devtools::document()
• [x] ran lintr::lint_package() and resolved all lint warnings and notes
• [x] ran styler::style_pkg() to make sure code matches the style guidelines
• [x] ran R-CMD CHECK and resolved all issues

potential new feature: when rebasing a plot, the connective lines for the points and control lines should be broken to clearly indicate a change of process has occured. This is similar to how qichart2 functions.

The default has been chose as TRUE, which is a departure from the standard excel tool, but I feel that this is a worthwhile departure as it is a limitation of charts in Excel.

[tomjemmett]

example of plot with broken lines:

[ThomUK]

I like this as an option.

What do you think about adding an option to break the limit lines, without breaking the process line?

Thinking about the questions I get from people on these charts, that would be a half-way house towards correctly representing the processes before and after a rebase as different (ie. the limit line breaks), without the potential confusion of having two process lines on the plot (ie. why are there two lines on the graph, can you put it back the way it was, please?)

[ThomUK]

Something like: break_limit_lines = TRUE (true by default) break_process_line = FALSE (true by default)

[tomjemmett]

we could add in extra options, though adding in extra options = increased complexity, it's whether we want to include that extra level of complexity. Though, I would always strongly argue against setting either option to FALSE as per points above.

Using two arguments will increase complexity of code, and therefore also the tests. At the moment it's implemented by setting in ggplot() the group aesthetic for all geoms (then, manually overriding the group argument for the target line). We would have to set the aesthetic on each geom manually with two arguments I think.

If we went for this approach I think the defaults should be

break_limit_lines = TRUE,
break_process_line = break_limit_lines

i.e. the default is to follow break_limit_lines. An alternative may be to have a check that says you can't have break_limit_lines == FALSE and break_process_line == TRUE (that just does not make sense :smile:) .

We could also take the approach that limit lines are always broken, there is no option to control that, but the argument controls whether to break the process line or not.

[tomjemmett]

the way I've chosen to handle this is by using a string for break_lines: one of "both" [DEFAULT], "limit", "process" or "none"

[tomjemmett]

added this change to the deviations vignette

[tomjemmett]

resolved merge conflicts (caused by #97), branch should now be complete, save for deciding whether the default option is what we want to use

[ThomUK]

I agree that "both" is a sensible default.
"none" would be used to mimic the excel behaviour.
I definitely have users who will want "limit" (to break limit lines but keep visual continuity of the process line).
I actually can't think of an occasion where I would use "process" (to break process line but not limit lines). No harm to keep it in, though.

[tomjemmett]

No I couldn't think of one either , other than it was sort of just as easy to add it in as it wasn't! If we are happy to merge this it's good to go