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

Allow PHP code to be included in the Phar template #111

Closed

Conversation

martin-rueegg
Copy link
Contributor

This PR allows to include PHP code in the resulting PHAR through new options

  • --head
  • --tail
  • --bootstrap

Requires

@martin-rueegg
Copy link
Contributor Author

@theseer Apparently you have missed a PHP5.6 incompatibility yourself:

public function getHomeDirectory(): string {

So after all, it is maybe indeed time to introduce 2.0 which is not compatible with 5.6 (or 5.3 as it says in the composer.json). 😊

Anyway, I've fixed this issue in this PR.

@theseer
Copy link
Owner

theseer commented Nov 18, 2023

I'd appreciate a PR for the 5.3/5.6 compatibility.

I'm still pondering whether I want to extend the template mechanism. I'm not sure I see the point in this extension? You can already specify custom templates as well as custom variable placeholders. Why would one need a custom head and tail?

@martin-rueegg
Copy link
Contributor Author

I'd appreciate a PR for the 5.3/5.6 compatibility.

What I meant is a version 2, that drops php 5.x compatibility altogether, raising the minimum required level to either 7.4 or event 8.2. I'm not sure, that this is what you'd want a PR to do, is it?

I'm still pondering whether I want to extend the template mechanism. I'm not sure I see the point in this extension? You can already specify custom templates as well as custom variable placeholders. Why would one need a custom head and tail?

Yes, I guess it's solvable with a custom template. And honestly, right now I can't recall what was the issue. It had to do with the head (the tail was just an addition, since I was adding the head) and the bootstrapping.

If you think it's not a good idea, I'm happy if you close the PR and I'll ask it to re-consider once I get back to the project where I was using it with a better use-case.

@theseer
Copy link
Owner

theseer commented Nov 18, 2023

I'd appreciate a PR for the 5.3/5.6 compatibility.

What I meant is a version 2, that drops php 5.x compatibility altogether, raising the minimum required level to either 7.4 or event 8.2. I'm not sure, that this is what you'd want a PR to do, is it?

No, you pointed out an issue with PHP 5.6 compatibility that I missed. It's really hard to restrict yourself back to 5.3 ;-p
So I was merely hinting that a dedicated PR to fix that would be nice rather than having it piggyback on this one ;)

I'm still pondering whether I want to extend the template mechanism. I'm not sure I see the point in this extension? You can already specify custom templates as well as custom variable placeholders. Why would one need a custom head and tail?

Yes, I guess it's solvable with a custom template. And honestly, right now I can't recall what was the issue. It had to do with the head (the tail was just an addition, since I was adding the head) and the bootstrapping.

If you think it's not a good idea, I'm happy if you close the PR and I'll ask it to re-consider once I get back to the project where I was using it with a better use-case.

I still can't see a good reason for the head/tail thing given that you have full control over the actual template as a whole.

I believe to understand what the boostrap thing does but I also fail to see why one would want the have that?
So, yes, I guess i'll close this. Sorry about that.

@theseer theseer closed this Nov 18, 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.

2 participants