From a5b43c9ac5419ce29ea4827a9858fa107f81ab68 Mon Sep 17 00:00:00 2001 From: Chris Recoskie Date: Wed, 9 May 2007 22:03:53 +0000 Subject: [PATCH] RESOLVED - bug 185409: ctrl-click is hardcoded to use C++ keywords https://bugs.eclipse.org/bugs/show_bug.cgi?id=185409 Patch from Mike Kucera --- .../cdt/ui/tests/text/HyperlinkTest.java | 190 ++++++++++++++ .../cdt/ui/tests/text/TextTestSuite.java | 3 + .../ui/editor/CElementHyperlinkDetector.java | 239 +++++++----------- .../search/actions/SelectionParseAction.java | 11 +- 4 files changed, 291 insertions(+), 152 deletions(-) create mode 100644 core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/HyperlinkTest.java diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/HyperlinkTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/HyperlinkTest.java new file mode 100644 index 00000000000..551153337d5 --- /dev/null +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/HyperlinkTest.java @@ -0,0 +1,190 @@ +/******************************************************************************* + * Copyright (c) 2000, 2007 IBM Corporation and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * IBM Corporation - initial API and implementation + *******************************************************************************/ +package org.eclipse.cdt.ui.tests.text; + +import junit.framework.TestCase; +import junit.framework.TestSuite; + +import org.eclipse.core.resources.IContainer; +import org.eclipse.core.resources.IFile; +import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.jface.text.IRegion; +import org.eclipse.jface.text.Region; +import org.eclipse.jface.text.hyperlink.IHyperlink; +import org.eclipse.jface.text.hyperlink.IHyperlinkDetector; +import org.eclipse.ui.texteditor.ITextEditor; + +import org.eclipse.cdt.core.dom.IPDOMManager; +import org.eclipse.cdt.core.model.ICContainer; +import org.eclipse.cdt.core.model.ICProject; +import org.eclipse.cdt.core.testplugin.CProjectHelper; + +import org.eclipse.cdt.internal.ui.editor.CEditor; +import org.eclipse.cdt.internal.ui.editor.CElementHyperlinkDetector; + + +public class HyperlinkTest extends TestCase { + + + private static final String CPP_FILE_NAME = "hyperlink_test_cpp.cpp"; + private static final String CPP_CODE = + "#include \n" + + "#define SOMEMACRO macro_token1 macro_token2 \n" + + "// COMMENT there should not be links inside of comments \n" + + "class Point { \n" + + " public: \n" + + " Point(); \n" + + " ~Point(); \n" + + " void set(int x, int y); \n" + + " int getX(); \n" + + " int getY(); \n" + + " private: \n" + + " int x, y; \n" + + "} \n" + + "int main() { \n" + + " char[] str = \"STRING LITERAL\"; \n" + + "} \n"; + + + private static final String C_FILE_NAME_1 = "hyperlink_test_c_1.c"; + private static final String C_CODE_1 = + "int main() { \n" + + " int class = 99; \n" + + "}"; + + + private static final String C_FILE_NAME_2 = "hyperlink_test_c_2.c"; + private static final String C_CODE_2 = + "#ifdef NOTDEF\n" + + " int nothere = 99; \n" + + "#else\n" + + " int itworks = 100; \n" + + "#endif\n"; + + + private ICProject project; + private ITextEditor editor; + + + public static TestSuite suite() { + return new TestSuite(HyperlinkTest.class); + } + + + private void setUpEditor(String fileName, String code) throws Exception { + super.setUp(); + project= CProjectHelper.createCCProject(super.getName(), "unused", IPDOMManager.ID_NO_INDEXER); + ICContainer cContainer= CProjectHelper.addCContainer(project, "src"); + IFile file= EditorTestHelper.createFile((IContainer)cContainer.getResource(), fileName, code, new NullProgressMonitor()); + + assertNotNull(file); + assertTrue(file.exists()); + editor = (CEditor)EditorTestHelper.openInEditor(file, true); + } + + protected void tearDown() throws Exception { + EditorTestHelper.closeEditor(editor); + CProjectHelper.delete(project); + } + + private IHyperlink[] getHyperlinks(int mouseOffset) { + IHyperlinkDetector detector = new CElementHyperlinkDetector(editor); + IRegion region = new Region(mouseOffset, 0); + return detector.detectHyperlinks(null, region, false); + } + + private void assertHyperlink(int mouseOffset, int linkStartOffset, int linkLength) { + IHyperlink[] links = getHyperlinks(mouseOffset); + assertNotNull(links); + assertEquals(1, links.length); + IRegion hyperlinkRegion = links[0].getHyperlinkRegion(); + assertEquals(linkStartOffset, hyperlinkRegion.getOffset()); + assertEquals(linkLength, hyperlinkRegion.getLength()); + } + + private void assertNotHyperlink(int mouseOffset) { + IHyperlink[] links = getHyperlinks(mouseOffset); + assertNull(links); + } + + + public void testHyperlinksCpp() throws Exception { + // entire include highlighted + setUpEditor(CPP_FILE_NAME, CPP_CODE); + + assertHyperlink(CPP_CODE.indexOf("#include") + 2, 0, "#include ".length()); + assertHyperlink(CPP_CODE.indexOf("") + 2, 0, "#include ".length()); + + // hovering over the whitspace inside an include still results in a hyperlink + assertHyperlink(CPP_CODE.indexOf("") - 1, 0, "#include ".length()); + + // no hyperlinks in macro bodies + assertNotHyperlink(CPP_CODE.indexOf("#define") + 1); + assertHyperlink(CPP_CODE.indexOf("SOMEMACRO"), CPP_CODE.indexOf("SOMEMACRO"), "SOMEMACRO".length()); + assertNotHyperlink(CPP_CODE.indexOf("macro_token1") + 1); + assertNotHyperlink(CPP_CODE.indexOf("macro_token2") + 1); + + // no hyperlinks for comments + assertNotHyperlink(CPP_CODE.indexOf("//") + 1); + assertNotHyperlink(CPP_CODE.indexOf("COMMENT") + 1); + + // no hyperlinks for keywords + assertNotHyperlink(CPP_CODE.indexOf("class") + 1); + assertNotHyperlink(CPP_CODE.indexOf("public") + 1); + assertNotHyperlink(CPP_CODE.indexOf("private") + 1); + assertNotHyperlink(CPP_CODE.indexOf("int x") + 1); + assertNotHyperlink(CPP_CODE.indexOf("char") + 1); + assertNotHyperlink(CPP_CODE.indexOf("void") + 1); + + // no hyperlinks for punctuation + assertNotHyperlink(CPP_CODE.indexOf("{")); + assertNotHyperlink(CPP_CODE.indexOf("}")); + assertNotHyperlink(CPP_CODE.indexOf("(")); + assertNotHyperlink(CPP_CODE.indexOf(")")); + assertNotHyperlink(CPP_CODE.indexOf(":")); + assertNotHyperlink(CPP_CODE.indexOf(";")); + + // no hyperlinks inside strings + assertNotHyperlink(CPP_CODE.indexOf("STRING") + 1); + assertNotHyperlink(CPP_CODE.indexOf("STRING") + 6); + assertNotHyperlink(CPP_CODE.indexOf("LITERAL") + 1); + + assertHyperlink(CPP_CODE.indexOf("Point {") + 1, CPP_CODE.indexOf("Point {"), "Point".length()); + assertHyperlink(CPP_CODE.indexOf("Point()") + 1, CPP_CODE.indexOf("Point()"), "Point".length()); + assertHyperlink(CPP_CODE.indexOf("~Point()") + 1, CPP_CODE.indexOf("~Point()"), "~Point".length()); + assertHyperlink(CPP_CODE.indexOf("set(") + 1, CPP_CODE.indexOf("set("), "set".length()); + assertHyperlink(CPP_CODE.indexOf("getX()") + 1, CPP_CODE.indexOf("getX()"), "getX".length()); + assertHyperlink(CPP_CODE.indexOf("getY()") + 1, CPP_CODE.indexOf("getY()"), "getY".length()); + } + + + public void testHyperlinksCKeywords() throws Exception { + setUpEditor(C_FILE_NAME_1, C_CODE_1); + + // 'class' is not a keyword in C, it should be hyperlinked + assertHyperlink(C_CODE_1.indexOf("class") + 1, C_CODE_1.indexOf("class"), "class".length()); + } + + + public void testHyperlinksInactiveCode() throws Exception { + setUpEditor(C_FILE_NAME_2, C_CODE_2); + + assertNotHyperlink(C_CODE_2.indexOf("#ifdef") + 2); + assertNotHyperlink(C_CODE_2.indexOf("#else") + 2); + assertNotHyperlink(C_CODE_2.indexOf("#endif") + 2); + + assertNotHyperlink(C_CODE_2.indexOf("nothere") + 1); + assertHyperlink(C_CODE_2.indexOf("itworks") + 1, C_CODE_2.indexOf("itworks"), "itworks".length()); + } + + + +} diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/TextTestSuite.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/TextTestSuite.java index ab7325a2876..ca600f79ba1 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/TextTestSuite.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/TextTestSuite.java @@ -49,5 +49,8 @@ public class TextTestSuite extends TestSuite { // basic editing tests addTest(BasicCEditorTest.suite()); + + // editor hyperlink tests + addTest(HyperlinkTest.suite()); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java index 09c16e5a3fa..93aa49a1977 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2005 QNX Software Systems and others. + * Copyright (c) 2000, 2007 QNX Software Systems and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,181 +7,122 @@ * * Contributors: * QNX Software Systems - Initial API and implementation + * IBM Corporation *******************************************************************************/ package org.eclipse.cdt.internal.ui.editor; -import java.util.Iterator; -import java.util.Set; - -import org.eclipse.cdt.core.parser.KeywordSetKey; -import org.eclipse.cdt.core.parser.ParserLanguage; -import org.eclipse.cdt.internal.core.parser.token.KeywordSets; +import org.eclipse.core.runtime.CoreException; import org.eclipse.jface.action.IAction; -import org.eclipse.jface.text.BadLocationException; -import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.IRegion; +import org.eclipse.jface.text.ITextSelection; import org.eclipse.jface.text.ITextViewer; import org.eclipse.jface.text.Region; import org.eclipse.jface.text.hyperlink.IHyperlink; import org.eclipse.jface.text.hyperlink.IHyperlinkDetector; import org.eclipse.ui.texteditor.ITextEditor; -public class CElementHyperlinkDetector implements IHyperlinkDetector{ +import org.eclipse.cdt.core.CCorePlugin; +import org.eclipse.cdt.core.dom.ast.IASTFileLocation; +import org.eclipse.cdt.core.dom.ast.IASTName; +import org.eclipse.cdt.core.dom.ast.IASTPreprocessorIncludeStatement; +import org.eclipse.cdt.core.dom.ast.IASTPreprocessorStatement; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.index.IIndex; +import org.eclipse.cdt.core.index.IIndexManager; +import org.eclipse.cdt.core.model.IWorkingCopy; +import org.eclipse.cdt.ui.CUIPlugin; + +import org.eclipse.cdt.internal.ui.search.actions.OpenDeclarationsAction; + +public class CElementHyperlinkDetector implements IHyperlinkDetector { private ITextEditor fTextEditor; - //TODO: Replace Keywords - //Temp. Keywords: Once the selection parser is complete, we can use - //it to determine if a word can be underlined - private Set fgKeywords; public CElementHyperlinkDetector(ITextEditor editor) { fTextEditor= editor; - fgKeywords = KeywordSets.getKeywords(KeywordSetKey.ALL,ParserLanguage.CPP); } - + + public IHyperlink[] detectHyperlinks(ITextViewer textViewer, IRegion region, boolean canShowMultipleHyperlinks) { if (region == null || canShowMultipleHyperlinks || !(fTextEditor instanceof CEditor)) return null; - IAction openAction= fTextEditor.getAction("OpenDeclarations"); //$NON-NLS-1$ + CEditor editor = (CEditor) fTextEditor; + int offset = region.getOffset(); + + IAction openAction= editor.getAction("OpenDeclarations"); //$NON-NLS-1$ if (openAction == null) return null; - // TODO: - //Need some code in here to determine if the selected input should - //be selected - the JDT does this by doing a code complete on the input - - //if there are any elements presented it selects the word - - int offset= region.getOffset(); - IDocument document= fTextEditor.getDocumentProvider().getDocument(fTextEditor.getEditorInput()); - - IRegion cregion = selectWord(document, offset); - if (cregion != null) { - return new IHyperlink[] {new CElementHyperlink(cregion, openAction)}; + // reuse the logic from Open Decl that recognizes a word in the editor + ITextSelection selection = OpenDeclarationsAction.selectWord(offset, editor); + if(selection == null) + return null; + + IWorkingCopy workingCopy = CUIPlugin.getDefault().getWorkingCopyManager().getWorkingCopy(editor.getEditorInput()); + + IIndex index; + try { + index = CCorePlugin.getIndexManager().getIndex(workingCopy.getCProject(), + IIndexManager.ADD_DEPENDENCIES | IIndexManager.ADD_DEPENDENT); + } catch(CoreException e) { + return null; + } + + try { + index.acquireReadLock(); + } catch (InterruptedException e) { + return null; + } + + try { + IASTTranslationUnit ast = + ASTProvider.getASTProvider().getAST(workingCopy, index, ASTProvider.WAIT_YES, null); + IASTName[] selectedNames = + workingCopy.getLanguage().getSelectedNames(ast, selection.getOffset(), selection.getLength()); + + IRegion linkRegion; + if(selectedNames.length > 0 && selectedNames[0] != null) { // found a name + linkRegion = new Region(selection.getOffset(), selection.getLength()); + } + else { // check if we are in an include statement + linkRegion = matchIncludeStatement(ast, selection); + } + + if(linkRegion != null) + return new IHyperlink[] { new CElementHyperlink(linkRegion, openAction) }; + + } catch(CoreException e) { + } finally { + index.releaseReadLock(); + } + + return null; + } + + + /** + * Returns the region that represents an include directive if one is found + * that matches the selection, null otherwise. + */ + private IRegion matchIncludeStatement(IASTTranslationUnit ast, ITextSelection selection) { + IASTPreprocessorStatement[] preprocs = ast.getAllPreprocessorStatements(); + for (int i = 0; i < preprocs.length; ++i) { + + if (!(preprocs[i] instanceof IASTPreprocessorIncludeStatement)) + continue; + + IASTFileLocation loc = preprocs[i].getFileLocation(); + if (loc != null + && loc.getFileName().equals(ast.getFilePath()) + && loc.getNodeOffset() < selection.getOffset() + && loc.getNodeOffset() + loc.getNodeLength() > selection.getOffset()) { + + return new Region(loc.getNodeOffset(), loc.getNodeLength()); + } } return null; } - private IRegion selectWord(IDocument document, int anchor) { - //TODO: Modify this to work with qualified name - - // fix for 95219, return null if the mouse is pointing to a non-java identifier part - try { - if (!Character.isJavaIdentifierPart(document.getChar(anchor))) { - return null; - } - } catch (BadLocationException e) { return null; } - - boolean isNumber=false; - try { - int offset= anchor; - char c; - char oldC='a'; // assume this is the first character - - while (offset >= 0) { - c= document.getChar(offset); - if (!Character.isJavaIdentifierPart(c)) { - if (Character.isDigit(oldC)) // if the first character is a digit, then assume the word is a number, i.e. 1e13, 0xFF, 123 - isNumber=true; - break; - } - oldC = c; - --offset; - } - - int start= offset; - - offset= anchor; - int length= document.getLength(); - - while (offset < length) { - c= document.getChar(offset); - if (!Character.isJavaIdentifierPart(c)) - break; - ++offset; - } - - int end= offset; - //Allow for new lines - if (start == end) - return new Region(start, 0); - - // don't select numbers only i.e. 0x1, 1e13, 1234 - if (isNumber) return null; - - String selWord = null; - String slas = document.get(start,1); - - // TODO more need to be added to this list as they are discovered - if (slas.equals("\n") || //$NON-NLS-1$ - slas.equals("\t") || //$NON-NLS-1$ - slas.equals(" ") || //$NON-NLS-1$ - slas.equals(">") || //$NON-NLS-1$ - slas.equals(".") || //$NON-NLS-1$ - slas.equals("(")) //$NON-NLS-1$ - { - - selWord =document.get(start+1, end - start - 1); - } - else{ - selWord =document.get(start, end - start); - } - //Check for keyword - if (isKeyWord(selWord)) - return null; - //Avoid selecting literals, includes etc. - char charX = selWord.charAt(0); - if (charX == '"' || - charX == '.' || - charX == '<' || - charX == '>') - return null; - - if (selWord.equals("#include")) //$NON-NLS-1$ - { - //get start of next identifier - - - int end2 = end; - - while (!Character.isJavaIdentifierPart(document.getChar(end2))){ - ++end2; - } - - while (end2 < length){ - c = document.getChar(end2); - - if (!Character.isJavaIdentifierPart(c) && - c != '.') - break; - ++end2; - } - - int finalEnd = end2; - selWord =document.get(start, finalEnd - start); - end = finalEnd + 1; - start--; - } - - return new Region(start + 1, end - start - 1); - - } catch (BadLocationException x) { - return null; - } - } - - private boolean isKeyWord(String selWord) { - Iterator i = fgKeywords.iterator(); - - while (i.hasNext()){ - String tempWord = (String) i.next(); - if (selWord.equals(tempWord)) - return true; - } - - return false; - } - - } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/SelectionParseAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/SelectionParseAction.java index 4750eb354d7..8e310fc10b4 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/SelectionParseAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/SelectionParseAction.java @@ -77,10 +77,15 @@ public class SelectionParseAction extends Action { StatusLineHandler.clearStatusLine(fSite); } - //TODO: Change this to work with qualified identifiers public ITextSelection getSelection( int fPos ) { - IDocumentProvider prov = ( fEditor != null ) ? fEditor.getDocumentProvider() : null; - IDocument doc = ( prov != null ) ? prov.getDocument(fEditor.getEditorInput()) : null; + return selectWord(fPos, fEditor); + } + + + //TODO: Change this to work with qualified identifiers + public static ITextSelection selectWord(int fPos, ITextEditor editor) { + IDocumentProvider prov = ( editor != null ) ? editor.getDocumentProvider() : null; + IDocument doc = ( prov != null ) ? prov.getDocument(editor.getEditorInput()) : null; if( doc == null ) return null;