Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Format no format #4657

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

Open
9swampy wants to merge 6 commits into GitTools:main
base: main
Choose a base branch
Loading
from 9swampy:FormatNoFormat
Open

Format no format #4657

9swampy wants to merge 6 commits into GitTools:main from 9swampy:FormatNoFormat

Conversation

@9swampy
Copy link
Contributor

@9swampy 9swampy commented Aug 13, 2025 •
edited
Loading

Fix legacy .NET composite format syntax

Description

Fixes legacy .NET format syntax {CommitsSinceVersionSource:0000;;''} that was outputting literal text instead of formatted values.

  • Added LegacyCompositeFormatter for semicolon-separated format sections
  • Updated ValueFormatter and RegexPatterns to support semicolons
  • Maintains compatibility with existing ?? syntax

Before: 6.13.54-gv6-CommitsSinceVersionSource-0000-----
After: 6.13.54-gv60002

Related Issue

Fixes #4654

Motivation and Context

Legacy .NET semicolon format syntax stopped working, breaking existing GitVersion configurations.

How Has This Been Tested?

Added comprehensive test suites:

  • BackwardCompatibilityTests.cs - 8 tests covering legacy format scenarios
  • Issue4654Tests.cs - 4 tests for the specific issue

All 12 new tests pass. No regression in existing tests.

Screenshots (if appropriate):

N/A

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Contributor Author

9swampy commented Aug 13, 2025

@phatcher #4654

@9swampy 9swampy force-pushed the FormatNoFormat branch 3 times, most recently from 3510bed to 8a331e5 Compare August 13, 2025 23:19
(?<member>[A-Za-z_][A-Za-z0-9_]*) # member/property name
(?: # Optional format specifier
:(?<format>[A-Za-z0-9\.\-,]+) # Colon followed by format string (no spaces, ?, or }), format cannot contain colon
:(?<format>[A-Za-z0-9\.\-,;'"]+) # Colon followed by format string (including semicolons and quotes)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RCA

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RCA?

Copy link

Copy link
Contributor Author

9swampy commented Aug 14, 2025

Apologies. Two passes is already more time than I had. The first pass was comfortable, the second's expanded in to assumptions that likely warrant a review & retrospect on regression pack coverage? Rollback & regroup the wise call?

@arturcic arturcic requested review from arturcic and asbjornu and removed request for arturcic August 14, 2025 04:43
Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work! 🎉 Thank you so much for jumping on this so quickly, @9swampy! ❤️

9swampy reacted with thumbs up emoji
Comment on lines +94 to +98
# Three-section format: positive;negative;zero
assembly-informational-format: "{Value:positive;negative;zero}"
# Two-section format: positive;negative
assembly-informational-format: "{Value:pos;neg}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these last two examples be modified to be more practical and real-world relevant, with a line showing the result, as in the first code example?

- **Third section**: Used for zero values (optional)
- **Empty quotes** (`''` or `""`) create empty output

**Mixed Syntax:** You can combine legacy semicolon syntax with modern `??` fallback syntax in the same template.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would ;; and ?? be combined? Can we give a code example for this as well?

9swampy reacted with laugh emoji
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the added tests! 🤩 Could we please have a test for the combination of ;; and ?? as well?

using GitVersion.Formatting;

namespace GitVersion.Tests.Formatting;
namespace GitVersion.Core.Tests.Formatting;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitVersion.Core should not be a namespace. :)

Suggested change
namespace GitVersion.Core.Tests.Formatting;
namespace GitVersion.Tests.Formatting;

9swampy reacted with thumbs up emoji

// Currently this will throw or behave unexpectedly
// Should either throw meaningful error or handle gracefully
Assert.Throws<ArgumentException>(() => template.FormatWith(semanticVersion, environment));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good practice to test the returned ArgumentException to ensure that the right guard clause is throwing it. This just ensures that an ArgumentException is being thrown anywhere by the code path, not necessarily by our own code.

using GitVersion.Formatting;

namespace GitVersion.Tests.Formatting;
namespace GitVersion.Core.Tests.Formatting;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace GitVersion.Core.Tests.Formatting;
namespace GitVersion.Tests.Formatting;

9swampy reacted with thumbs up emoji
using GitVersion.Formatting;

namespace GitVersion.Tests.Formatting;
namespace GitVersion.Core.Tests.Formatting;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace GitVersion.Core.Tests.Formatting;
namespace GitVersion.Tests.Formatting;

9swampy reacted with thumbs up emoji
(?<member>[A-Za-z_][A-Za-z0-9_]*) # member/property name
(?: # Optional format specifier
:(?<format>[A-Za-z0-9\.\-,]+) # Colon followed by format string (no spaces, ?, or }), format cannot contain colon
:(?<format>[A-Za-z0-9\.\-,;'"]+) # Colon followed by format string (including semicolons and quotes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RCA?

Copy link
Contributor

@9swampy You mention "modern ?? fallback syntax" - what do you mean by that, AFAIK ?? is just the null coalescing operation and nothing to do with string formatting?

Copy link
Contributor Author

9swampy commented Sep 17, 2025

Honestly I'm embarrassed I've not got back to fixing this. It's been maniac at my day job and I've maxed the AI tooling out repeatedly so haven't had capacity to revisit.

The reason to call that out now is to answer the question; don't get hung up on the modern quip. AIism.

I will get a poke at the fix as soon as I can.

Copy link
Contributor

@9swampy No worries - reason I was asking was to see if I could do the same idea of suppressing the commit number when it's zero via a different syntax

Copy link
Member

@9swampy any updates on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asbjornu asbjornu asbjornu requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ISSUE] Incorrect output for assembly-informational-version

AltStyle によって変換されたページ (->オリジナル) /