Fixes #168 Diff not done on real text provided by the AI #172
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| package io.github.jeddict.ai.components; | ||
|
|
||
| import com.github.javaparser.ParserConfiguration; | ||
| import com.github.javaparser.Range; | ||
| import com.github.javaparser.StaticJavaParser; | ||
| import com.github.javaparser.ast.CompilationUnit; | ||
| import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; | ||
|
|
@@ -582,7 +583,7 @@ public int getAllCodeEditorCount() { | |
| return count; | ||
| } | ||
|
|
||
| public void getParseCodeEditor(List<FileObject> context) { | ||
| StaticJavaParser.getParserConfiguration().setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_21); | ||
| Map<JEditorPane, Map<String, String>> editorMethodSignCache = new HashMap<>(); | ||
| Map<JEditorPane, Map<String, String>> editorMethodCache = new HashMap<>(); | ||
|
|
@@ -602,13 +603,9 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| // can probably be combined into one | ||
| // 4. before changing this logic, a unit test to drive the collection | ||
| // logic should be first created. | ||
| // 5. it is not fully clear why here the context files are parsed with | ||
| // StaticJavaParser instead of using NetBeans JavaSource | ||
| // 6. editors are processed only if they contain java code; this is a | ||
| // limitation that can probably be removed as we should be able to | ||
| // do some diffs even for files other than java | ||
| // | ||
|
|
@@ -633,13 +630,10 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| // | ||
| Map<String, Integer> fileMethodSignatures = new HashMap<>(); | ||
| for (MethodDeclaration method : methods) { | ||
| if (isAnonymousInnerMethod(method)) { | ||
| continue; // Skip anonymous inner class method | ||
| } | ||
| String signature = buildMethodSignature(method); | ||
| fileMethodSignatures.put(signature, method.toString().length()); | ||
| } | ||
|
|
||
|
|
@@ -649,7 +643,7 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| // | ||
| Map<String, Long> fileMethods = new HashMap<>(); | ||
| for (MethodDeclaration method : methods) { | ||
| if (isAnonymousInnerMethod(method)) { | ||
|
Collaborator
There was a problem hiding this comment. I've realised now we are not counting and saving the counter, but saving the position. Can you please update the comment too? |
||
| continue; // Skip anonymous inner class method | ||
| } | ||
| String methodName = method.getNameAsString(); | ||
|
|
@@ -676,72 +670,55 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| if (parentPanel.getComponent(i) instanceof JEditorPane) { | ||
| JEditorPane editorPane = (JEditorPane) parentPanel.getComponent(i); | ||
| if (editorPane.getEditorKit().getContentType().equals(JAVA_MIME)) { | ||
| Map<String, String> cachedMethodSignatures = editorMethodSignCache.computeIfAbsent(editorPane, ep -> { | ||
| Map<String, String> snippetSignatures = new HashMap<>(); | ||
| String editorText = editorPane.getText(); | ||
| String[] lines = editorText.split("\n"); | ||
| try { | ||
| // check if snippet is method otherwise throw exception | ||
| extractMethod(snippetSignatures, editorText); | ||
| } catch (Exception e1) { | ||
| try { | ||
| CompilationUnit aiCu = StaticJavaParser.parse(editorText); | ||
| extractClasses(aiCu, lines, snippetSignatures, editorText); | ||
| extractMethods(aiCu, lines, snippetSignatures); | ||
| } catch (Exception e2) { | ||
| try { | ||
|
Collaborator
There was a problem hiding this comment. What is this case for? Maybe you can add a comment to explain the purpose. However please review how it can be refactored taking into account the try..catch above: if failed above, it would fail here too.
Member
Author
There was a problem hiding this comment. I think this check was added to handle inner classes or partial code snippets. Anyway, my next focus will be on the patch-based way of suggesting code, so it can auto applied and reverted, specifically #159 |
||
| CompilationUnit aiCu = StaticJavaParser.parse(editorText); | ||
| extractClasses(aiCu, lines, snippetSignatures, editorText); | ||
| if (aiCu.getTypes().isNonEmpty()) { | ||
| snippetSignatures.put(aiCu.getType(0).getNameAsString(), editorText); | ||
| } | ||
| } catch (Exception e3) { | ||
| // ignore | ||
| } | ||
| } | ||
| } | ||
| return snippetSignatures; | ||
| }); | ||
|
|
||
| Map<String, String> cachedMethods = editorMethodCache.computeIfAbsent(editorPane, ep -> { | ||
| Map<String, String> snippetSignatures = new HashMap<>(); | ||
| String editorText = editorPane.getText(); | ||
| String[] lines = editorText.split("\n"); | ||
| try { | ||
| extractMethod(snippetSignatures, editorText); | ||
| } catch (Exception e) { | ||
| try { | ||
| CompilationUnit aiCu = StaticJavaParser.parse(editorText); | ||
| extractMethods(aiCu, lines, snippetSignatures); | ||
| } catch (Exception e1) { | ||
| try { | ||
| CompilationUnit aiCu = StaticJavaParser.parse(editorText); | ||
| if (aiCu.getTypes().isNonEmpty()) { | ||
| snippetSignatures.put(aiCu.getType(0).getNameAsString(), editorText); | ||
| } | ||
| } catch (Exception e2) { | ||
| // ignore | ||
| } | ||
| } | ||
| } | ||
| return snippetSignatures; | ||
| }); | ||
|
|
||
| // | ||
|
|
@@ -754,7 +731,7 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| // | ||
| try { | ||
| int menuCreationCount = 0; | ||
| for (Map.Entry<String, Integer> signature : fileMethodSignatures.entrySet()) { | ||
| if (createEditorPaneMenus(fileObject, signature.getKey(), signature.getValue(), editorPane, cachedMethodSignatures)) { | ||
| menuCreationCount++; | ||
| } | ||
|
|
@@ -780,43 +757,40 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| System.out.println("Error parsing file: " + fileObject.getName() + " - " + e.getMessage()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Add the menus to the relevant editor's context menu | ||
| */ | ||
| public void attachMenusToEditors() { | ||
| // Create the menu to diff with selected text | ||
| for (int i = 0; i < parentPanel.getComponentCount(); i++) { | ||
| if (parentPanel.getComponent(i) instanceof JEditorPane editorPane) { | ||
| menuItems.computeIfAbsent(editorPane, k -> new ArrayList<>()); | ||
| if (JAVA_MIME.equals(editorPane.getEditorKit().getContentType())) { | ||
| JMenuItem diffMethodItem = new JMenuItem("Diff with Selected Snippet"); | ||
| diffMethodItem.addActionListener(e -> SwingUtilities.invokeLater(() -> { | ||
| JTextComponent currenteditor = EditorRegistry.lastFocusedComponent(); | ||
| String currentSelectedText = currenteditor.getSelectedText(); | ||
| final StyledDocument currentDocument = (StyledDocument) currenteditor.getDocument(); | ||
| DataObject currentDO = NbEditorUtilities.getDataObject(currentDocument); | ||
| if (currentDO != null) { | ||
| FileObject focusedfile = currentDO.getPrimaryFile(); | ||
| if (focusedfile != null && currentSelectedText != null && !currentSelectedText.trim().isEmpty()) { | ||
| diffActionWithSelected(currentSelectedText, focusedfile, editorPane); | ||
| } else { | ||
| JOptionPane.showMessageDialog(null, "Please select text in the source editor."); | ||
| } | ||
| } else { | ||
| JOptionPane.showMessageDialog(null, "Please select text in the source editor."); | ||
| } | ||
| })); | ||
| menuItems.get(editorPane).add(diffMethodItem); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| for (Map.Entry<JEditorPane, List<JMenuItem>> entry : menuItems.entrySet()) { | ||
| JPopupMenu mainMenu = menus.get(entry.getKey()); | ||
| if (mainMenu != null) { | ||
|
|
@@ -836,7 +810,59 @@ public int getParseCodeEditor(List<FileObject> context) { | |
| mainMenu.add(methodMenu); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private boolean isAnonymousInnerMethod(MethodDeclaration method) { | ||
| return method.getParentNode().isPresent() && method.getParentNode().get() instanceof ObjectCreationExpr; | ||
| } | ||
|
|
||
| private String buildMethodSignature(MethodDeclaration method) { | ||
| return method.getNameAsString() + "(" | ||
| + method.getParameters().stream() | ||
| .map(param -> param.getType().asString()) | ||
| .collect(Collectors.joining(",")) + ")"; | ||
| } | ||
|
|
||
| private void extractClasses(CompilationUnit aiCu, String[] lines, | ||
| Map<String, String> snippetSignatures, String source) { | ||
| List<ClassOrInterfaceDeclaration> classDecls = aiCu.findAll(ClassOrInterfaceDeclaration.class); | ||
| int classesCount = classDecls.size(); | ||
|
|
||
| for (ClassOrInterfaceDeclaration classDecl : classDecls) { | ||
| if (classesCount == 1 || classDecl.isPublic()) { | ||
|
Collaborator
There was a problem hiding this comment. shouldn't this be outside the loop and then you loop only if classesCount is >1? or at least break from the loop?
Member
Author
There was a problem hiding this comment. Break isn’t needed since only one public class is allowed, and we still need to process other non-public top-level classes. |
||
| snippetSignatures.put(classDecl.getNameAsString(), source); | ||
| } else { | ||
| classDecl.getRange().ifPresent(range -> { | ||
| String classSource = extractSource(lines, range.begin.line, range.end.line); | ||
| snippetSignatures.put(classDecl.getNameAsString(), classSource); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private void extractMethods(CompilationUnit aiCu, String[] lines, Map<String, String> snippetSignatures) { | ||
| List<MethodDeclaration> aiMethods = aiCu.findAll(MethodDeclaration.class); | ||
| for (MethodDeclaration aiMethod : aiMethods) { | ||
| String signature = buildMethodSignature(aiMethod); | ||
| aiMethod.getRange().ifPresent(range -> { | ||
| String methodSource = extractSource(lines, range.begin.line, range.end.line); | ||
| snippetSignatures.put(signature, methodSource); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| private void extractMethod(Map<String, String> snippetSignatures, String editorText) { | ||
| MethodDeclaration aiMethod = StaticJavaParser.parseMethodDeclaration(editorText); | ||
| String signature = aiMethod.getNameAsString(); | ||
| snippetSignatures.put(signature, editorText); | ||
| } | ||
|
|
||
| private String extractSource(String[] lines, int startLine, int endLine) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (int i = startLine - 1; i <= endLine - 1; i++) { | ||
| sb.append(lines[i]).append("\n"); | ||
| } | ||
| return sb.toString(); | ||
| } | ||
|
|
||
| private boolean createEditorPaneMenus(FileObject fileObject, String signature, Integer bodyLength, JEditorPane editorPane, Map<String, String> cachedMethodSignatures) { | ||
|
|
@@ -869,11 +895,7 @@ private boolean createEditorPaneMenus(FileObject fileObject, String signature, I | |
| JMenuItem diffMethodItem = new JMenuItem("Diff " + menuSubText + fileObject.getName()); | ||
| diffMethodItem.addActionListener(e -> { | ||
| SwingUtilities.invokeLater(() -> { | ||
| diffAction(classSignature, fileObject, signature, editorPane, cachedMethodSignatures); | ||
| }); | ||
| }); | ||
| if (fileObject.getName().equals(signature)) { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 am actually wondering why we cache the length; in any case, is the use of the parsed text correct here?
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.
By comparing the method body length, it detects whether any changes were suggested by the AI, so the method itself can be compared in the Diff panel instead of the entire class.
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.
ok got it. shouldn't a hash be more accurate? Of course anyway not on the parsed text.
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.
Length is just a quick heuristic, but a hash would be more accurate for detecting actual changes.