Rename function widget #9959
Rename function widget #9959
Conversation
farmaazon
added
CI: No changelog needed
|
Technically, it's ready on GUI side, but still does not work because of #9960. I could not find any better name for WidgetFunctionName (literally, the widget displays not only name, but also path/self argument). Any suggestions are welcome. |
app/gui2/src/stores/graph/index.ts
Outdated
| mapOk(proj.modulePath, normalizeQualifiedName) | ||
| : Err('Uknown current module name') | ||
| if (!modulePath?.ok) return modulePath | ||
| console.log(ptr.module, modulePath.value) |
| toastError.show(msg) | ||
| } | ||
| if (result.ok) code.value = `${ctx.selfIdent}.${result.value}` | ||
| toastError.reportError(result, 'Applying AI prompt failed') |
| newName, | ||
| ) | ||
| if (!refactorResult.ok) return refactorResult | ||
| displayedName.value = refactorResult.value.newName |
There was a problem hiding this comment.
Not supre sure if it is OK to modify the local state after await. This introduces a race condition with other changes happening in the document. I think this should only be applied in case displayedName wasn't changed in the mean time, or not at all. After all, the name change should be propagated through document update anyway.
app/gui2/src/stores/graph/index.ts
Outdated
| } | ||
|
|
||
| function getMethodAst(ptr: MethodPointer, module?: Ast.Module): Result<Ast.Function> { | ||
| const topLevel = (module ?? syncModule.value)?.root() |
There was a problem hiding this comment.
Instead of using a fallback here, I think the module argument should be required and syncModule.value should simply be used as a parameter when it's appropriate (i.e. in computed above).
There was a problem hiding this comment.
This module cannot be just any module - it should always be a syncModule or one of its edited states. I'd rather rename parameter name to edit and keep it optional. It's published as method of "graph" storage, so it should be obvious of what module we want to take a definition.
app/gui2/src/util/toast.ts
Outdated
| reportError<T, E>(result: Result<T, E>, preamble?: string) { | ||
| if (!result.ok) { | ||
| const msg = result.error.message(preamble) | ||
| console.error(msg) | ||
| this.show(msg) | ||
| } | ||
| return result |
There was a problem hiding this comment.
Not sure about this. Putting the error check condition into this function forces the preamble string to be always computed, and it is sort of awkward to use when you have to check for ok everywhere anyway. I think this should rather receive a ResultError as argument and just handle printing it. We can always combine this with a function like mapError on results.
…/rename-functions
…/rename-functions
…/rename-functions
Pull Request Description
Fixes #9790
Adds a new widget type which allows renaming the called function - if we are able to do it.
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.