search

xsawyerx / ref-util-rewriter

Rewrite your code to use Ref::Util
Other
1 stars 2 forks source link

GH #1: experimental attempt for quoted evals #2

Closed atoomic closed 5 years ago

atoomic commented 8 years ago

This commit is not complete but here as proof of concept/attempt, and will work for the most basic cases. The content read from literal should be escaped correctly, once updated

PPI::Token::Quote::* API is fuzzy and the tricky part is to replace the element this can be done by $sib->insert_before( $p ); $sib->delete but the creation of the element is not trivial.

# '' - PPI::Token::Quote::Single
# "q{}" - PPI::Token::Quote::Literal
# "" - PPI::Token::Quote::Double
# "qq{}" - PPI::Token::Quote::Interpolate

Maybe use the internal of the PPI Quote element ?

xsawyerx commented 8 years ago

I would like to change the following:

atoomic commented 5 years ago

@xsawyerx changes suggested performed and rebased on top of the travis changes from PR #7 ready for another review

xsawyerx commented 5 years ago

Hey, :)

I see there are still whitespace changes which makes me more difficult for me to review.

I can review this but I will have to undo the whitespace changes first. If you want it reviewed sooner, you will need to remove the whitespace changes yourself in a following commit.

atoomic commented 5 years ago

@xsawyerx I will indeed strip the whitespaces in a follow up commit, I thought this was already the case, I ve missed some...

From the GitHub UI there is a Hide whitespace changes option which can be helpful but this was not my intent to add these extra spaces

atoomic commented 5 years ago

this should be ready for another review with extra spaces removed.. this is going to conflict with the other pending PR but should be easy to resolve this is probably not the most important change to commit first, I would give priority to #8

xsawyerx commented 5 years ago

The other reason I request to undo the whitespace is that I purposefully align my code in a certain way. I didn't want it formatted differently.

xsawyerx commented 5 years ago

I realize I missed all the other PRs. Sorry about that. I was about to merge yours but I moved the entire eval stuff to another subroutine and I need to make sure it doesn't contradict other PRs. Maybe I should merge them first.

xsawyerx commented 5 years ago

Cherry-picked the right commit, cleaned up what I still didn't like, and merged. Thanks! :+1: