Skip to content

Comments

Added support for runtime expressions embedded in string literals#290

Merged
darrelmiller merged 5 commits intovnextfrom
dm/callbackkey
Jul 13, 2018
Merged

Added support for runtime expressions embedded in string literals#290
darrelmiller merged 5 commits intovnextfrom
dm/callbackkey

Conversation

@darrelmiller
Copy link
Member

Addresses issue #287

/// </summary>
public class CompositeExpression : RuntimeExpression
{
private readonly string template;
Copy link
Contributor

Choose a reason for hiding this comment

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

template [](start = 32, length = 8)

not sure if template is the right word here... maybe "originalExpression" ?

{
private readonly string template;
private Regex expressionPattern = new Regex("{(?<exp>[^}]+)");
/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, please add one newline for readability


foreach (var item in matches.Cast<Match>())
{
var value = item.Groups["exp"].Captures.Cast<Capture>().First().Value;
Copy link
Contributor

Choose a reason for hiding this comment

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

value [](start = 20, length = 5)

nit, call this runtimeExpression instead of value

@PerthCharern
Copy link
Contributor

Copy link
Contributor

@PerthCharern PerthCharern left a comment

Choose a reason for hiding this comment

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

:shipit:

@darrelmiller
Copy link
Member Author

@PerthCharern I will make these changes. How do feel about releasing 1.1, rather than me back porting these changes to 1.0?

@PerthCharern
Copy link
Contributor

PerthCharern commented Jul 9, 2018 via email

@darrelmiller darrelmiller merged commit b454f27 into vnext Jul 13, 2018
@darrelmiller darrelmiller deleted the dm/callbackkey branch July 13, 2018 18:55
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