mapstruct / mapstruct-idea

An IntelliJ IDEA plugin for working with MapStruct
Other
141 stars 38 forks source link

Set caret position to empty source attribute after quick fix #120

Closed thunderhook closed 1 year ago

thunderhook commented 1 year ago

This PR fixes #28.

The test handles the case with and without surrounding @Mappings({...}) annotation.

filiphr commented 1 year ago

Thanks a lot for this fix @thunderhook. I had a look at it and it looks great. I have only one ask, I think that we need to move the caret only for the "Add unmapped target property". With this changes it moves the caret for the ignore fixes as well.

Please let us know if you do not have time to finish this and we'll drive it home.

thunderhook commented 1 year ago

Yes, you're right.

I would like to finish this PR if you don't mind. I would recommend to extend UnmappedTargetPropertyFix with a field to control whether the caret has to be moved or not.

What do you think? Is a boolean enough for that case? Or do we need something like an enum or a BiConsumer< Project, PsiElement > to make it more expressive?

        return new UnmappedTargetPropertyFix(
            method,
            message,
            MapStructBundle.message( "intention.add.unmapped.target.property" ),
            new UnmappedTargetPropertyFixAnnotationSupplier( method, target ),
            true
            // vs. MapstructAnnotationUtils::moveCaretToEmptySourceAttribute
            // vs. CaretBehaviour.NONE|MOVE_TO_SOURCE|etc.
        );
filiphr commented 1 year ago

I would like to finish this PR if you don't mind.

Of course I don't mind. I only offered to take it over in case you don't have the time. I've been really busy and it has taken me a long time to get back to you.

Adding a boolean to the UnmappedTargetPropertyFix is fine for now. In case we need more options in the future, we can always change it.

thunderhook commented 1 year ago

Of course I don't mind. I only offered to take it over in case you don't have the time. I've been really busy and it has taken me a long time to get back to you.

Thanks and no problem. I really appreciate that. 🙂 I really enjoy contributing to this plugin and currently have time for it.

I added two tests for ignoring a single and all properties. If there is anything else, please let me know.

filiphr commented 1 year ago

Thanks @thunderhook