Skip to content
This repository has been archived by the owner on Mar 12, 2022. It is now read-only.

Custom blade directives #103

Merged
merged 3 commits into from Feb 1, 2017
Merged

Conversation

adelf
Copy link
Contributor

@adelf adelf commented Jan 26, 2017

I've implemented completion for blade custom directives. But php autocompletion for parameters doesn't work. Should we do something with it, or Blade plugin is better place to implement it?

Copy link
Owner

@Haehnchen Haehnchen left a comment

Choose a reason for hiding this comment

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

kudos to this implementation

@Override
public void visitElement(PsiElement element) {

try {
Copy link
Owner

Choose a reason for hiding this comment

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

please remove this try catch and add additional checks if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added try{ only for finally section. Without finally I had to replace each { return; } to { super.visitElement(element); return; } :) Ofc I can change it if you don't like try{ here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

return;
}

if (method.getContainingClass() == null || !availableClasses.contains(method.getContainingClass().getPresentableFQN())) {
Copy link
Owner

Choose a reason for hiding this comment

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

i am normally normalizing classes and stripping \ first. so looks also valid here because of using add("Blade") and add("\Blade")


public class BladeCustomDirectivesVisitor extends PsiRecursiveElementVisitor {

private Set<String> availableClasses = new HashSet<>();
Copy link
Owner

Choose a reason for hiding this comment

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

a STATIC variable would be better because then there is no need to rebuild availableClasses every time

return;
}

Method method = Symfony2InterfacesUtil.getMultiResolvedMethod(methodReference);
Copy link
Owner

Choose a reason for hiding this comment

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

you are not allowed to use resolve in index process. the index process make it possible to resolve such methods so as you can see there is a conflict in intellij api usage here. :)

i think this works in you case because getMultiResolvedMethod does not leave the current file document. if you extend more classes in PhpClass itself you will run it this issue.

you are just allowed to use document. so try to match against some method name or class usage. for example https://github.com/Haehnchen/idea-php-symfony2-plugin/blob/master/src/fr/adrienbrault/idea/symfony2plugin/stubs/indexes/AnnotationRoutesStubIndex.java. it collect all @Route php annotations


String directiveName = psiElement.getText().substring(1);

FileBasedIndexImpl.getInstance().getFilesWithKey(
Copy link
Owner

Choose a reason for hiding this comment

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

FileBasedIndex would be right Impl is just the implementation

@Haehnchen
Copy link
Owner

@adelf which parameter are not working? in Blade file itself or PHP files

@adelf
Copy link
Contributor Author

adelf commented Jan 26, 2017

I mean @datetime(..php autocompletion doesn't works here..) but it works in @if(..) for example.
Thanks for explaining about resolve issue :) I spent 1-2 hours with this thing. I'll try to fix it.

@Haehnchen
Copy link
Owner

Haehnchen commented Jan 26, 2017

resolve index took weeks to understand for me :)

@datetime is tricky but looks possible for me by now. i hope PhpStorm 2017 brings some improvements. a starting point(?):
https://github.com/Haehnchen/idea-php-laravel-plugin/blob/master/src/de/espend/idea/laravel/blade/BladeDirectiveReferences.java#L97

@adelf
Copy link
Contributor Author

adelf commented Jan 29, 2017

Sometthing is wrong with downloading phpstorm versions

@Haehnchen
Copy link
Owner

... that is annoying me at lot, fixed in master an merged. 👍

@Haehnchen Haehnchen merged commit 5f3c996 into Haehnchen:master Feb 1, 2017
@adelf adelf deleted the custom-blade-directives branch February 19, 2017 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants