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 unsetProperty in hooks #1308

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gabsource
Copy link

@gabsource gabsource commented Jan 25, 2022

Summary

This PR add options to ignore some properties :

  • via the unsetProperty in a hook

It is useful to prevent PhpDoc generation for properties that should be used via a model cast only through a custom class.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • [] Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@mfn
Copy link
Collaborator

mfn commented May 3, 2022

Wouldn't be the point of the hooks that we don't add configs for every use-case on earth?

Like: sure, let's add the hook method but not the config?

@gabsource
Copy link
Author

Wouldn't be the point of the hooks that we don't add configs for every use-case on earth?

Like: sure, let's add the hook method but not the config?

Sure. Found it more convenient (less verbose) but indeed it's for a specific use case and may pollute the config file.

@gabsource gabsource changed the title Ignore column properties via config and allow unset properties in hooks Add unsetPropertie in hooks May 3, 2022
@gabsource gabsource changed the title Add unsetPropertie in hooks Add unsetProperty in hooks May 3, 2022
@@ -830,7 +837,7 @@ public function setMethod($name, $type = '', $arguments = [], $comment = '')

public function unsetMethod($name)
{
unset($this->methods[strtolower($name)]);
unset($this->methods[$name]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if we should change this…

It's from the initial implementation https://github.com/barryvdh/laravel-ide-helper/pull/1198/files#diff-2df8ee218526540f71334c651990edfbfe5345cc53a521b0259c874ff24abd9aR766

ping @jenga201 if you remember why?

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.

None yet

2 participants