Skip to content

Conversation

@rolfheij-sil
Copy link
Collaborator

@rolfheij-sil rolfheij-sil commented Aug 4, 2022

…erse/chapter values


This change is Reviewable

@rolfheij-sil rolfheij-sil requested a review from tombogle August 4, 2022 14:33
// chapter and verse is optional in regex, make it 1 if not given
if (string.IsNullOrEmpty(chapter))
chapter ="1";
else if (chapter.Length > 3 && Regex.IsMatch(chapter, @"^\d+$"))
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect that the actual resulting behavior here would not be what a user would expect. I think maybe if the chapter or verse portion is longer than 3 digits, we would just want to treat it like any other invalid pasted reference. It's most likely a typo or something and any one of the extra digits could be the errant one.

// chapter and verse is optional in regex, make it 1 if not given
if (string.IsNullOrEmpty(chapter))
chapter ="1";
else if (chapter.Length > 3 && Regex.IsMatch(chapter, @"^\d+$"))
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove the Regex.IsMatch then. If it's longer than 3 characters, it's invalid no matter what.

/// <summary>Fired when any part of the reference is invalid</summary>
public event EventHandler InvalidReferenceEntered;
/// <summary>Fired when any part of the pasted reference is invalid</summary>
public event EventHandler InvalidReferencePasted;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a new public event, it should have an entry in the ### Added section of CHANGELOG.md (and it should have +semver:minor in the comment)

Copy link
Contributor

@tombogle tombogle left a comment

Choose a reason for hiding this comment

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

See comment about adding entry to CHANGELOG

@rolfheij-sil rolfheij-sil merged commit 01d2ad5 into master Aug 4, 2022
@rolfheij-sil rolfheij-sil deleted the 94_PTX-22668 branch August 4, 2022 17:18
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.

3 participants