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

Fix issues with field, hasfield, isfield post ID parsing #68

Closed
wants to merge 3 commits into from

Conversation

EHLOVader
Copy link

Switching to is_numeric to identify ID values from strings, since the blade template is always parsed as a string and is_string wasn't catching IDs.

This may however need more to really fix it. This will handle any cases when someone passes in a number directly, but does not handle variables or function calls, which might happen.

Fixes #44
Fixes #63

Switching to is_numeric to identify ID values from strings, since the blade template is always parsed as a string and is_string wasn't catching IDs.

Fixes Log1x#44
Fixes Log1x#63
Checks for numeric, get_the_ID,  $post->ID, or get_queried_object_id()
@EHLOVader
Copy link
Author

I have added an attempt at identifying wp post ID retrieval methods which might be used with get_field* directives.

@erip2
Copy link

erip2 commented Mar 22, 2022

This is really needed. Also fixes an issue I opened some days ago before I saw this PR: #70

@Log1x Sorry, is this package still maintained?

@EHLOVader
Copy link
Author

@erip2 It was. I may have slowed this down, not wanting to introduce bugs and hoping that it could be tested by others who had issues I've mentioned above.

Log1x was open to merge

It sounds like you may have confirmed it worked with composer-patches perhaps?

@erip2
Copy link

erip2 commented Mar 24, 2022

@EHLOVader I didn't use composer-patches.

I've only created my own directives with the changes you've made here and I've used those but I would like better to use the package ones instead of creating custom ones.

@EHLOVader
Copy link
Author

Wondering if this looks good to merge? If it does can you merge it and add #hacktoberfest-accepted label to it? Thanks!

@2gen
Copy link

2gen commented May 25, 2023

This PR fails when you pass a variable to the @field directive. Take the following use-case:

@foreach($postids as $id)
{{ $id }}'s Title: @field('title', $id)
@endforeach

It should be noted that if you are passing variables as your $id you are limited to the following only:

  • get_the_ID()
  • $post->ID
  • get_queried_object_id()

i.e.. works:

@foreach($posts as $post)
{{ $post->ID }}'s Title: @field('title', $post->ID)
@endforeach

@EHLOVader
Copy link
Author

That is correct.

It also accepts an explicit number as noted in the documentation.

@field('text', 1)

I think that $id makes sense to add to the list of options. I tried to find any ways that someone might be able to get an ID and pass it through in the view. Before this I don't think it worked for any value (variables, functions, or numbers) because it was using is_string and the first go at this fix was just checking is_numeric which worked with all the examples in the docs, but didn't work when I wanted to use a variable.

It would be good to add these notes to the docs, or maybe it needs rewritten to work differently. Not sure what can reliably be done since it isn't running the php just parsing the string and generating php from the blade input so you have no clue what output of a function will be, or the type of the variable.

I think this presented issues with string manipulations and arrays before too.

@Log1x Log1x mentioned this pull request Jul 29, 2023
@Log1x
Copy link
Owner

Log1x commented Jul 29, 2023

Superseded by #82

@Log1x Log1x closed this Jul 29, 2023
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.

@hasfield render argument in wrong place @hasfield compiled incorrectly
4 participants