-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
ArrayIndexOutOfBoundsException for duplicate dependencies #61
Comments
That is fine. IntelliJ holds the model; it is updated by Reimport. Removing nodes from the GUI is only for UX. Maybe it would be better to exclude it only from the right node: 443534d |
Ah ok. Haven't looked at Intellij's maven structure. If those "UI bugs" are due to Intellij's not-updated model of that pom.xml then why is my approach not enough? My approach seems reasonable because it's near the exception and fixes that exception. I don't understand how your referenced commit should fix the bug. It doesn't, btw. |
It seems wrong to exclude it from 2 places and then remove it from the GUI only on one. The solution should not be to simply get rid of the exception (your fix), or to remove it in the GUI on two places, but to exclude it only from the right dependency (my fix). Now I fixed it a bit more, but now I am worried about false negatives. Maybe it will be better to do the filtering in several steps until only one pom dependency matches. |
Ah, I see. I'd love to add some integration-tests but still failing :( That would help a lot. |
Hi,
when I add a dependency twice, like
and then I exclude something in the first dependency, like
jmespath-java
then
krasa.mavenhelper.analyzer.action.ExcludeDependencyAction#exclude
callsdependencyExcluded()
twice and the second call produces this exceptionMy simple approach was to move
dependencyExcluded()
out of the for-loop to just after itThis kind of works for the pom.xml in text mode
but the UI is refreshed and displays
jmespath-java
still for the second depAfter hitting "Refresh UI" it even displays
jmespath-java
for the first dep as well (see first screenshot).I need to "Reimport" and then it's fine.
I didn't make a pull request because I'm not sure whether my change would be appropriate because of the UI bugs afterwards.
The text was updated successfully, but these errors were encountered: