Skip to content

Convenience methods for TextInputModelSubmission.Value conversions#2393

Open
ecrocombe wants to merge 6 commits intoDSharpPlus:masterfrom
VereTech-Gaming:pw/feat/textinputmodelsubmission-valuetypes
Open

Convenience methods for TextInputModelSubmission.Value conversions#2393
ecrocombe wants to merge 6 commits intoDSharpPlus:masterfrom
VereTech-Gaming:pw/feat/textinputmodelsubmission-valuetypes

Conversation

@ecrocombe
Copy link
Contributor

Summary

Proposed convenience methods for parsing string Value of TextInputModelSubmission to T.

Details

Some convenience methods for parsing string Value.

Notes

Maybe these should be placed in extensions? will leave that up to your discretion.

Copy link
Member

@akiraveliara akiraveliara left a comment

Choose a reason for hiding this comment

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

without commenting on the general desire for the change (cc @Plerx2493 ), these should be instance methods and we'd not want to use regions if not absolutely necessary

also, left a comment

Comment on lines +148 to +155
foreach (string i in submission.Values)
{
if (i.TryGetValueAs<T>(out T? value))
{
yield return value;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this should have an error case, probably an exception

…ption on SelectMenuModalSubmission.GetValuesAs, to align with TextInputModalSubmission.GetValueAs on parsing errors.

namespace DSharpPlus.Extensions;

internal static class StringExtentions
Copy link
Member

Choose a reason for hiding this comment

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

upon thinking about this further, i think we absolutely should not extend string even internally, since it'll pollute the symbol space for every string in the entire project. use a static helper method instead, please

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.

2 participants