Hi all,
I see some new tests failing - testResumeEarlySyntaxError - testResumeLateSyntaxError This is entirely my fault because of the "fix" of exception handling. Indeed, I have not rearmed the exception stack in resume: action, and I think I should have done so. However, I see that SyntaxErrorNotification>>resume: launch a new compilation entirely. This sounds un-necessary to me because we already have ReparseAfterSourceEditing for that purpose... (again I'm the original author of that, Christoph only improved it) IMO, the purpose is not really to resume: but rather to retry compilation using newSource. We (I ?) have used resume: only because notify:at: does signal a SyntaxErrorNotification, then immediately retries if the exception resumes. (notification := SyntaxErrorNotification cue: (cue copy source: (source contents asText copyReplaceFrom: location to: location - 1 with: messageText); yourself) doitFlag: doitFlag errorMessage: string location: location) signal. notification tryNewSourceIfAvailable But if the handler does retryWithNewSource: newSource instead of resume: newSource, then we just have to let retryWithNewSource perform a self resignalAs: ReparseAfterSourceEditing new. I think that it's the right solution, rather than bending the semantics of resume: (the argument of resume: is normally the value that should be returned to the sender of signal...). So if you see some changes of SyntaxErrorNotification in trunk, you'll know a bit more about it (I'm not sure whether I can redact a commit message with that level of explanation). Nicolas |
Le ven. 16 avr. 2021 à 20:35, Nicolas Cellier
<[hidden email]> a écrit : > > Hi all, > I see some new tests failing > - testResumeEarlySyntaxError > - testResumeLateSyntaxError > > This is entirely my fault because of the "fix" of exception handling. > Indeed, I have not rearmed the exception stack in resume: action, and > I think I should have done so. > > However, I see that SyntaxErrorNotification>>resume: launch a new > compilation entirely. > This sounds un-necessary to me because we already have > ReparseAfterSourceEditing for that purpose... > (again I'm the original author of that, Christoph only improved it) > > IMO, the purpose is not really to resume: but rather to retry > compilation using newSource. > We (I ?) have used resume: only because notify:at: does signal a > SyntaxErrorNotification, then immediately retries if the exception > resumes. > > (notification := SyntaxErrorNotification > cue: (cue copy > source: (source contents asText > copyReplaceFrom: location > to: location - 1 > with: messageText); > yourself) > doitFlag: doitFlag > errorMessage: string > location: location) signal. > notification tryNewSourceIfAvailable > > But if the handler does retryWithNewSource: newSource instead of > resume: newSource, then we just have to let retryWithNewSource perform > a self resignalAs: ReparseAfterSourceEditing new. > > I think that it's the right solution, rather than bending the > semantics of resume: (the argument of resume: is normally the value > that should be returned to the sender of signal...). > > So if you see some changes of SyntaxErrorNotification in trunk, you'll > know a bit more about it > (I'm not sure whether I can redact a commit message with that level of > explanation). > > Nicolas On the other hand, I cannot entirely remove tryNewSourceIfAvailable, because it is still the resolution path for SyntaxError window... So I have two alternatives: resignalAs: or resume: (with reactivateHandlers fix). retryWithNewSource: source "Retry the compilation with new source code. Assume that there is already a handler of ReparseAfterSourceEditing on the sender stack. All I have to do then, is to resignal myself as such and let the existing handler do its business." ^ self resignalAs: (ReparseAfterSourceEditing new withNewSource: source) Or: retryWithNewSource: source "Retry the compilation with new source code. Assume: - that the signallerContext will tryNewSourceIfAvailable - and the presence of a handler of ReparseAfterSourceEditing on the sender stack" newSource := source. ^ self resume: self defaultResumeValue I'm still in favour of naming it #retryWithNewSource: rather than resume: because super resume: has another semantic than we shall better not bend. |
Le ven. 16 avr. 2021 à 21:52, Nicolas Cellier
<[hidden email]> a écrit : > > Le ven. 16 avr. 2021 à 20:35, Nicolas Cellier > <[hidden email]> a écrit : > > > > Hi all, > > I see some new tests failing > > - testResumeEarlySyntaxError > > - testResumeLateSyntaxError > > > > This is entirely my fault because of the "fix" of exception handling. > > Indeed, I have not rearmed the exception stack in resume: action, and > > I think I should have done so. > > > > However, I see that SyntaxErrorNotification>>resume: launch a new > > compilation entirely. > > This sounds un-necessary to me because we already have > > ReparseAfterSourceEditing for that purpose... > > (again I'm the original author of that, Christoph only improved it) > > > > IMO, the purpose is not really to resume: but rather to retry > > compilation using newSource. > > We (I ?) have used resume: only because notify:at: does signal a > > SyntaxErrorNotification, then immediately retries if the exception > > resumes. > > > > (notification := SyntaxErrorNotification > > cue: (cue copy > > source: (source contents asText > > copyReplaceFrom: location > > to: location - 1 > > with: messageText); > > yourself) > > doitFlag: doitFlag > > errorMessage: string > > location: location) signal. > > notification tryNewSourceIfAvailable > > > > But if the handler does retryWithNewSource: newSource instead of > > resume: newSource, then we just have to let retryWithNewSource perform > > a self resignalAs: ReparseAfterSourceEditing new. > > > > I think that it's the right solution, rather than bending the > > semantics of resume: (the argument of resume: is normally the value > > that should be returned to the sender of signal...). > > > > So if you see some changes of SyntaxErrorNotification in trunk, you'll > > know a bit more about it > > (I'm not sure whether I can redact a commit message with that level of > > explanation). > > > > Nicolas > > On the other hand, I cannot entirely remove tryNewSourceIfAvailable, > because it is still the resolution path for SyntaxError window... > So I have two alternatives: resignalAs: or resume: (with > reactivateHandlers fix). > > retryWithNewSource: source > "Retry the compilation with new source code. > Assume that there is already a handler of > ReparseAfterSourceEditing on the sender stack. > All I have to do then, is to resignal myself as such and let the > existing handler do its business." > > ^ self resignalAs: (ReparseAfterSourceEditing new withNewSource: source) > > Or: > > retryWithNewSource: source > "Retry the compilation with new source code. > Assume: > - that the signallerContext will tryNewSourceIfAvailable > - and the presence of a handler of ReparseAfterSourceEditing on > the sender stack" > > newSource := source. > ^ self resume: self defaultResumeValue > > I'm still in favour of naming it #retryWithNewSource: rather than > resume: because super resume: has another semantic than we shall > better not bend. I opted for the second solution. I hope that the commit message explains the reason for renaming sufficiently clearly. |
Free forum by Nabble | Edit this page |