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

Add Path.Lookup #70

Merged
merged 16 commits into from
Mar 1, 2023
Merged

Add Path.Lookup #70

merged 16 commits into from
Mar 1, 2023

Conversation

StefH
Copy link
Collaborator

@StefH StefH commented Feb 20, 2023

No description provided.

@StefH StefH added the enhancement New feature or request label Feb 20, 2023
@StefH StefH self-assigned this Feb 20, 2023
@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

Merging #70 (5e16b8c) into master (d6fe6df) will increase coverage by 0.99%.
The diff coverage is 68.13%.

❗ Current head 5e16b8c differs from pull request most recent head 2651c94. Consider uploading reports for the commit 2651c94 to get more accurate results

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   43.98%   44.97%   +0.99%     
==========================================
  Files          40       43       +3     
  Lines        2626     2679      +53     
  Branches      362      367       +5     
==========================================
+ Hits         1155     1205      +50     
- Misses       1372     1375       +3     
  Partials       99       99              
Impacted Files Coverage Δ
...c/Handlebars.Net.Helpers.Core/Utils/ObjectUtils.cs 0.00% <0.00%> (ø)
...ebars.Net.Helpers.DynamicLinq/JObjectExtensions.cs 57.25% <ø> (+4.62%) ⬆️
...ers.DynamicLinq/Models/DynamicPropertyWithValue.cs 80.00% <ø> (ø)
...ars.Net.Helpers/Extensions/MethodBaseExtensions.cs 25.92% <31.57%> (+12.88%) ⬆️
....Helpers.DynamicLinq/Helpers/DynamicLinqHelpers.cs 18.40% <76.19%> (+5.36%) ⬆️
src/Handlebars.Net.Helpers/HandlebarsHelpers.cs 82.07% <90.90%> (+1.17%) ⬆️
.../Handlebars.Net.Helpers/Helpers/DateTimeHelpers.cs 66.66% <100.00%> (ø)
src/Handlebars.Net.Helpers/Helpers/PathHelpers.cs 100.00% <100.00%> (ø)
...ars.Net.Helpers/PathStructure/PathResolverProxy.cs 100.00% <100.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@oformaniuk
Copy link
Member

How is this different from build-in lookup helper?

@StefH
Copy link
Collaborator Author

StefH commented Feb 21, 2023

@zjklee
Ah. I understand. There's a builtin helper.
But that helper does not allow for 3 arguments with a defaultValue.

So a change could be like:

using HandlebarsDotNet.PathStructure;

namespace HandlebarsDotNet.Helpers
{
    public sealed class LookupReturnHelperDescriptor : IHelperDescriptor<HelperOptions>
    {
        public PathInfo Name { get; } = "lookup";

        public object Invoke(in HelperOptions options, in Context context, in Arguments arguments)
        {
            if (arguments.Length != 2 && arguments.Length != 3)
            {
                throw new HandlebarsException("{{lookup}} helper must have two or three arguments");
            }

            var segment = ChainSegment.Create(arguments[1]);

            var defaultValueIfNotFound = arguments.Length == 3 ? arguments[2] : UndefinedBindingResult.Create(segment);
            
            return !options.TryAccessMember(arguments[0], segment, out var value)
                ? defaultValueIfNotFound
                : value;
        }

        public void Invoke(in EncodedTextWriter output, in HelperOptions options, in Context context, in Arguments arguments)
        {
            output.Write(Invoke(options, context, arguments));
        }
    }
}

@oformaniuk
Copy link
Member

@StefH , looks good to me 🙂

@StefH
Copy link
Collaborator Author

StefH commented Feb 21, 2023

@zjklee Shall I create a PR for Handlebars.Net ?

@StefH
Copy link
Collaborator Author

StefH commented Feb 21, 2023

@zjklee See PR Handlebars-Net/Handlebars.Net#542

@StefH StefH changed the title Add Dictionary.Lookup Add Path.Lookup Feb 21, 2023
@StefH StefH mentioned this pull request Mar 1, 2023
@StefH StefH merged commit 52a23df into master Mar 1, 2023
@StefH StefH deleted the dictionary-lookup branch March 1, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants