-
Notifications
You must be signed in to change notification settings - Fork 17
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
Language server based on LSP #20
Conversation
}) | ||
} | ||
|
||
def startStageProcess(fileToVerify: String): Option[String] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, we do not need the concept of stages anymore as it is obsolete. Can we easily carve out all the code that is related to this feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. this is something that should reworked in conjunction with the settings. I have now removed all the code that relates to this concept.
} | ||
} | ||
|
||
// private def startVerificationTimeout(verificationCount: Int) = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the plan for this? Ideally, we shouldn't have any code that is commented out without explanations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer needed.
import scala.collection.mutable | ||
import scala.collection.mutable.ArrayBuffer | ||
|
||
object Coordinator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember reading about this concept in your thesis. Is it described somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is descibed in a sub-section of the Language server chapter. (1.3.3. in the draft version)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR.
What parts of the LSP frontend are currently tested, and what parts are not?
Please address my comments, and then we merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. This PR looks good from my point of view.
No description provided.