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

Wrapped unsafe TemplateStructuredTextViewerConfiguration.scalaConfiguration in Option. #179

Merged
merged 1 commit into from
Nov 19, 2013

Conversation

jhaberstro
Copy link
Contributor

This prevents possible null pointer exceptions in other SSE component who create instances of this class, such as the Maven POM Editor.

Fixes #176

…ration in option.

This prevents possible null pointer exceptions in other SSE component who create instances of this class, such as the Maven POM Editor.

Fixes scala-ide#176
@skyluc
Copy link
Member

skyluc commented Aug 8, 2013

The part I'm missing is why this code is invoked by the Maven plugin. The changes seem to patch the problem, but I would prefer to get in a state where our code doesn't leak in other people plugins.
Do you have any idea of what is happening? I had a quick at the configuration of the plugin, but it seem to register the SourceViewer only to Play Template context.

@jhaberstro
Copy link
Contributor Author

I really don't know, unfortunately. My only guess would be that our content type "org.scalaide.play2.templateSource" somehow steps on the feat of other content types that have .html as a possible file type (implying that whatever content type that the pom editor is using also includes .html as a possible extension). I do know of some existing wonkiness between the HTML editor and the template editor, and I had spent a day trying to figure out how disambiguate based on the fact that our extension type is .scala.html, but I came to the conclusion (based on other peoples experience on the internet and my failed attempts) that it's just not possible (or, it is possible, but nobody in the world knows the solution :P). So, when a .xml or .html file is create, in certain extension points, our template extension points take precedence,

@jhaberstro
Copy link
Contributor Author

Can this be merged as a temporary fix to the problem, and I'll file an issue for the deeper problem?

@dragos
Copy link
Member

dragos commented Aug 14, 2013

This LGTM, but I think @skyluc has a better understanding now. So I leave it to him to decide if this workaround is a good idea or not.

@skyluc
Copy link
Member

skyluc commented Aug 14, 2013

I think #182 fixes the problem of having the Play code called by Maven editor.

@jhaberstro
Copy link
Contributor Author

Yes, I agree that #182 fixes the "root" issue. However, I believe the code introduced in this PR is still an improvement (safeguards a possible null pointer exception in the case of bad initialization) that is worth incorporating in its own right.

@dragos
Copy link
Member

dragos commented Aug 19, 2013

@skyluc WDYT? Should we merge this as well?

@dotta
Copy link
Member

dotta commented Nov 5, 2013

@skyluc @dragos What's the take on this?

@ghprb-bot
Copy link

Can one of the admins verify this patch?

@dotta
Copy link
Member

dotta commented Nov 11, 2013

ok to test

@ghprb-bot
Copy link

Test FAILed.
Refer to this link for build results: https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-play-validator/2/

@dotta
Copy link
Member

dotta commented Nov 11, 2013

test this please

@ghprb-bot
Copy link

Test FAILed.
Refer to this link for build results: https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-play-validator/3/

@dotta
Copy link
Member

dotta commented Nov 11, 2013

test this please

@ghprb-bot
Copy link

Test FAILed.
Refer to this link for build results: https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-play-validator/4/

@dotta
Copy link
Member

dotta commented Nov 11, 2013

test this please

@ghprb-bot
Copy link

Test FAILed.
Refer to this link for build results: https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-play-validator/5/

@dotta
Copy link
Member

dotta commented Nov 11, 2013

test this please

@ghprb-bot
Copy link

Test PASSed.
Refer to this link for build results: https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-play-validator/6/

@huitseeker
Copy link
Member

retest this please

@huitseeker
Copy link
Member

Also, @skyluc I didn't see your comment on whether we should consider this one in its own right

@ghprb-bot
Copy link

Test PASSed.
Refer to this link for build results: https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-play-validator/7/

@huitseeker
Copy link
Member

Anyway, this LGTM !

@skyluc
Copy link
Member

skyluc commented Nov 19, 2013

👍

skyluc added a commit that referenced this pull request Nov 19, 2013
Wrapped unsafe TemplateStructuredTextViewerConfiguration.scalaConfiguration in Option.
@skyluc skyluc merged commit d234e22 into scala-ide:master Nov 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 0.4.0.v-2_10-201308021043-8c62ead breaks Maven POM Editor
7 participants