max-minors lists too many commits in last listed tag

[ite-klass]

When I use --max-minors 1 I see a lot more commits within the latest tag changes than I expect. There seems to be a logic issue. It lists commits of previous tags.

Looking at the output for values 1 2 and 3, it looks like it always lists commits that should be ommitted in the last displayed tag.

If it makes a difference, I am using a tag.. range parameter too.

convco changelog ^
  --max-minors 1 ^
  --merges ^
  --first-parent ^
  --include-hidden-sections ^
  v8.14.0.. ^
  > CHANGELOG.md

System (please complete the following information):

• OS: Windows 10
• Version: convco 0.3.12

[hdevalke]

It looks like there is a difference between vx.y.z.. and vx.y.z..HEAD.

[hdevalke]

I could reproduce it on this repo:

These are the expected commits for v0.3.0.

❯ git log --oneline v0.2.3..v0.3.0 
724bb2b (tag: v0.3.0) release v0.3.0
88dfa4d chore: introduce cargo-release
76b8ff4 feat(commit): validate commit message created by `convco commit`
302fc6e ci: run `cargo test` in Github Actions
dc03118 feat: Allow a custom scope regex in the configuration
8c728a8 fix(version)!: prioritize `--major` `--minor` `--patch` over `--bump`
01c9ea9 feat(changelog): Add option to set custom template directory in `.versionrc`
ae24b00 chore: update dependencies

However:

convco changelog --max-minors 1 --no-links --no-wrap --include-hidden-sections v0.2.1..v0.3.0  |rg 'v\d\.\d\.\d|[0-9a-f]{7}'
## v0.3.0 (2020-08-23)
* **commit:** validate commit message created by `convco commit` (76b8ff4)
* Allow a custom scope regex in the configuration (dc03118), closes #8
* **changelog:** Add option to set custom template directory in `.versionrc` (01c9ea9), closes #3
* relax regex for scope to allow -/_ as separator (61ee293)
* allow a scope to contain numbers (768492a)
* **changelog:** find host, owner and repository from the origin url (2675fcb)
* **version:** prioritize `--major` `--minor` `--patch` over `--bump` (8c728a8)
* introduce cargo-release (88dfa4d)
* run `cargo test` in Github Actions (302fc6e)
* update dependencies (ae24b00)
* bump version to 0.2.3 (89c5ccb)
* bump version to 0.2.2 (ba01120)
* **changelog:** Refactor changelog command for better readability and performance (817a39e)```

It contains the changes of v0.2.3 and v0.2.2 but not of v0.2.1. :thinking: 

[hdevalke]

The issue is here: https://github.com/convco/convco/blob/master/src/cmd/changelog.rs#L323

                    .chain(Some(Rev(rev_stop, None)))

where rev_stop is the left side of your range v8.14.0..
I probably have to refactor the way the iterator is build to walk through the versions to fix this issue

[ctron]

I see the same issue. A lot more change entries, which do not belong into the tag.

[hdevalke]

This should fix it, but I have to test it a bit more

diff --git a/src/cmd/changelog.rs b/src/cmd/changelog.rs
index 80e6260..391f2e0 100644
--- a/src/cmd/changelog.rs
+++ b/src/cmd/changelog.rs
@@ -315,9 +315,6 @@ impl ChangelogCommand {
                             .into_iter()
                             .rev()
                             .take_while(|v| v.tag != rev_stop)
-                            .take_while(|v| v.version.major() >= stop_at_major)
-                            .take_while(|v| v.version.minor() >= stop_at_minor)
-                            .take_while(|v| v.version.patch() >= stop_at_patch)
                             .map(|v| v.into()),
                     )
                     .chain(Some(Rev(rev_stop, None)))
@@ -326,6 +323,16 @@ impl ChangelogCommand {
                 for w in iter.windows(2) {
                     let from = &w[0];
                     let to = &w[1];
+                    match from {
+                        Rev(_, Some(v))
+                            if v.major() < stop_at_major
+                                || v.minor() < stop_at_minor
+                                || v.patch() < stop_at_patch =>
+                        {
+                            break
+                        }
+                        _ => (),
+                    }
                     let context = transformer.transform(from, to)?;
                     if !self.skip_empty || !context.context.commit_groups.is_empty() {
                         writer.write_template(&context)?;

[hdevalke]

@ctron @ite-klass Would you be able to test #93 on your repos before I merge it?

[ctron]

Looks good to me.

[ite-klass]

LGTM for the bug test case; the list does not include too many commits anymore