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

Remove empty lines from changelog before parsing #1097

Merged
merged 1 commit into from
Jan 22, 2023

Conversation

sandrofigo
Copy link
Contributor

Remove empty lines from the already read changelog lines before parsing it further in ReadReleaseNotes, similar to the already existing code in ExtractChangelogSectionNotes.

ReadReleaseNotes now accepts an AbsolutePath instead of a string as input, but it should make no difference to the public interface as AbsolutePath has an implicit conversion operator for string.

Closes #1094

I confirm that the pull-request:

  • Follows the contribution guidelines
  • Is based on my own work
  • Is in compliance with my employer

@matkoch
Copy link
Member

matkoch commented Jan 22, 2023

Changing the signature is not a good idea :) I reverted that part, otherwise thanks for the PR!

@matkoch matkoch merged commit 331c922 into nuke-build:develop Jan 22, 2023
@sandrofigo sandrofigo deleted the 1094-fix-changelog-parsing branch January 22, 2023 22:09
@matkoch
Copy link
Member

matkoch commented Feb 8, 2023

Unfortunately this breaks in the following scenario:

    AbsolutePath ChangelogFile => WorkingDirectory / "CHANGELOG.md";
    ChangeLog Changelog => ReadChangelog(ChangelogFile);

    Target UpdateChangelog => _ => _
        .OnlyWhenStatic(() => Changelog.Unreleased == null)
        .Executes(() =>
        {
            var products = new[]
            {
                HasReSharperImplementation ? "ReSharper" : null,
                HasRiderImplementation ? "Rider" : null
            }.WhereNotNull();

            var changelogLines = ChangelogFile.ReadAllLines().ToList();
            changelogLines.InsertRange(
                Changelog.ReleaseNotes[^1].StartIndex,
                collection: new[]
                {
                    $"## vNext",
                    $"- Added support for {products.Join(" and ")} {ReSharperShortVersion}",
                    string.Empty
                });
            ChangelogFile.WriteAllLines(changelogLines);
        });

I guess the start indexes don't align anymore.

@sandrofigo
Copy link
Contributor Author

Seems like I have overlooked that, since in my mind the 'StartIndex' property was private the whole time 😓
The property StartIndex refers to the index in the changelog file (without empty lines). I see that the start index is now off due to this PR.
I think we would need to rewrite the changelog parsing a bit more to have the line indices preserved.
This would also affect the unit tests (mostly the ones using Verify) proposed in #1122, so they would need to be adapted as well after this.

Is the StartIndex and EndIndex something vital that needs to be public or could we potentially make it private?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ReadReleaseNotes() method throws exception when changelog file contains empty lines
2 participants