scala.tools.eclipse.ScalaPresentationCompiler: ask() or askOption()?

[Simon Schäfer]

I found a bug which results in an exception. The code which produces the
bug is executed inside of askOption() - thus, it is impossible to catch
it with our test suite in a simple way.

Now I want to know if I shall change askOption() to ask() to let the
exception fly to scala-ide to handle it with a test+fix or shall I only
commit a fix and no test and no change of askOption()?

The ticket: https://www.assembla.com/spaces/scala-ide/tickets/1001222
The line of code which produces the bug:
https://github.com/scala-ide/scala-ide/blob/master/org.scala-ide.sdt.core/src/scala/tools/eclipse/semantichighlighting/classifier/SafeSymbol.scala#L108

[iulian dragos]

Hi Simon,

On Thu, Aug 23, 2012 at 7:19 PM, Simon Schäfer <ma...@antoras.de> wrote:

I found a bug which results in an exception. The code which produces the bug is executed inside of askOption() - thus, it is impossible to catch it with our test suite in a simple way.

Indeed, the assumption is that code passed to 'askOption' should not throw exceptions. In hindsight, this design is a bit restrictive, but it has the advantage no special care is needed to make sure that threads don't end up abruptly. In the short term, I'd investigate why that exception is thrown and fix it. In the long run, we'll change the `askOption` call to use `Response` variables that return an instance of `Either`, so proper error checkin can still be done. 

 

Now I want to know if I shall change askOption() to ask() to let the exception fly to scala-ide to handle it with a test+fix or shall I only commit a fix and no test and no change of askOption()?

I'd keep `askOption` for the moment.

iulian

 

The ticket: https://www.assembla.com/spaces/scala-ide/tickets/1001222
The line of code which produces the bug: https://github.com/scala-ide/scala-ide/blob/master/org.scala-ide.sdt.core/src/scala/tools/eclipse/semantichighlighting/classifier/SafeSymbol.scala#L108

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

[Simon Schäfer]

Ok, I fixed the problem and did a PR: https://github.com/scala-ide/scala-ide/pull/189