TASK-058: outline for C++ with CRLF line endings, macros and #if blocks
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
5cc11d8420
commit
b663f24009
@ -81,7 +81,9 @@ class CodeDocument(
|
||||
fun rememberCodeDocument(path: RepositoryPath, text: String): CodeDocument {
|
||||
val colors = rememberCodeSyntaxColors()
|
||||
val language = remember(path) { syntaxLanguageForPath(path) }
|
||||
val lines = remember(text) { text.split("\n") }
|
||||
// Files written on Windows end their lines with "\r\n". The "\r" is not part of the line: left
|
||||
// in, it is an invisible last character that no declaration pattern expects (TASK-058).
|
||||
val lines = remember(text) { text.split("\n").map { it.removeSuffix("\r") } }
|
||||
val plain = remember(path, lines) {
|
||||
CodeDocument(path, language, lines, lines.map(::AnnotatedString), emptyList(), lines.maxOfOrNull(String::length) ?: 0)
|
||||
}
|
||||
|
||||
@ -34,13 +34,27 @@ private val OUTLINE_LANGUAGES = setOf(SyntaxLanguage.CSHARP, SyntaxLanguage.CPP,
|
||||
*/
|
||||
fun extractSymbols(lines: List<String>, language: SyntaxLanguage): List<CodeSymbol> {
|
||||
if (!supportsOutline(language)) return emptyList()
|
||||
val code = blankCommentsAndStrings(lines, language)
|
||||
val source = lines.map { it.removeSuffix("\r") }
|
||||
val code = blankCommentsAndStrings(source, language)
|
||||
val symbols = mutableListOf<Pending>()
|
||||
val scopes = ArrayDeque<Scope>()
|
||||
// How deep the scopes were where each open `#if` began.
|
||||
val conditionals = ArrayDeque<Int>()
|
||||
var awaitingBody: Pending? = null
|
||||
var linesAwaited = 0
|
||||
|
||||
code.forEachIndexed { index, line ->
|
||||
// Only one branch of an `#if` is compiled, but all of them are read here. When each branch
|
||||
// opens the same block (`#ifdef A` / `if (x) {` / `#else` / `if (y) {` / `#endif`), counting
|
||||
// both braces would leave a scope that never closes and hide the rest of the file. So each
|
||||
// further branch starts from the depth the first one started from.
|
||||
when (directiveOf(source[index])) {
|
||||
"if", "ifdef", "ifndef" -> conditionals.addLast(scopes.size)
|
||||
"else", "elif", "elifdef", "elifndef" -> conditionals.lastOrNull()?.let { depth ->
|
||||
while (scopes.size > depth) scopes.removeLast()
|
||||
}
|
||||
"endif" -> conditionals.removeLastOrNull()
|
||||
}
|
||||
val enclosing = scopes.lastOrNull()?.kind
|
||||
val canDeclare = enclosing == null || enclosing == ScopeKind.NAMESPACE || enclosing == ScopeKind.TYPE
|
||||
if (awaitingBody != null && ++linesAwaited > MAX_LINES_AWAITING_BODY) awaitingBody = null
|
||||
@ -142,6 +156,14 @@ private class Pending(val found: Found, val line: Int, val depth: Int) {
|
||||
|
||||
private class Found(val name: String, val label: String, val kind: SymbolKind)
|
||||
|
||||
private val DIRECTIVE = Regex("^\\s*#\\s*([a-z]+)")
|
||||
|
||||
private fun directiveOf(line: String): String? = DIRECTIVE.find(line)?.groupValues?.get(1)
|
||||
|
||||
// `DECLARE_MESSAGE_MAP()`, `Q_OBJECT`-style: written like a call, named in capitals, and -- unlike
|
||||
// a declaration -- often not followed by `;`.
|
||||
private val MACRO_NAME = Regex("[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)*")
|
||||
|
||||
private fun blankCommentsAndStrings(lines: List<String>, language: SyntaxLanguage): List<String> =
|
||||
tokenizeLines(lines, language).mapIndexed { index, tokens ->
|
||||
val characters = lines[index].toCharArray()
|
||||
@ -225,6 +247,9 @@ private fun cppDeclaration(line: String, trimmed: String, firstWord: String, ins
|
||||
// `Name(...)` is a constructor or a macro-declared function. A free-standing call
|
||||
// (`DoThing(x);` at namespace level) is not valid C++, so it cannot be one.
|
||||
val isConstructorLike = insideType || "::" in qualified
|
||||
// A macro invocation in a class body is not a member, and as it has no `;` it would
|
||||
// also swallow the declaration on the next line while a body was waited for.
|
||||
if (!hasReturnType && "::" !in qualified && name.length > 1 && MACRO_NAME.matches(name)) return null
|
||||
if (name !in STATEMENT_WORDS && (hasReturnType || isConstructorLike)) {
|
||||
return Found(name, name + parameterTypes(match.groupValues[2]), SymbolKind.METHOD)
|
||||
}
|
||||
|
||||
@ -99,6 +99,64 @@ class SymbolOutlineTest {
|
||||
assertTrue(symbols.none { it.name == "public" || it.name == "private" })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `lines ending in a carriage return are read like any other`() {
|
||||
val header = "class A\r\n{\r\npublic:\r\n\tvoid F(int a);\r\n};\r\n".split("\n")
|
||||
val source = "void A::F(int a)\r\n{\r\n}\r\n".split("\n")
|
||||
val sharp = "public class A\r\n{\r\n public void F()\r\n {\r\n }\r\n}\r\n".split("\n")
|
||||
|
||||
assertEquals(listOf("A", "F(int)"), extractSymbols(header, SyntaxLanguage.CPP).map { it.label })
|
||||
assertEquals(listOf("F(int)"), extractSymbols(source, SyntaxLanguage.CPP).map { it.label })
|
||||
assertEquals(listOf("A", "F()"), extractSymbols(sharp, SyntaxLanguage.CSHARP).map { it.label })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a macro in a class body is not a method and does not hide the declaration after it`() {
|
||||
val lines = """
|
||||
class CMainDlg : public CDialogEx
|
||||
{
|
||||
DECLARE_DYNAMIC(CMainDlg)
|
||||
public:
|
||||
CMainDlg(CWnd* pParent = NULL);
|
||||
virtual ~CMainDlg();
|
||||
DECLARE_MESSAGE_MAP()
|
||||
public:
|
||||
afx_msg void OnBnClickedOk();
|
||||
};
|
||||
""".trimIndent().lines()
|
||||
|
||||
assertEquals(
|
||||
listOf("CMainDlg", "CMainDlg(CWnd*)", "~CMainDlg()", "OnBnClickedOk()"),
|
||||
extractSymbols(lines, SyntaxLanguage.CPP).map { it.label },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a block opened in both branches of a conditional is counted once`() {
|
||||
val lines = """
|
||||
BOOL CMainDlg::PreTranslateMessage(MSG* pMsg)
|
||||
{
|
||||
#ifdef A
|
||||
if (x) {
|
||||
#else
|
||||
if (y) {
|
||||
#endif
|
||||
}
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
int After(int a)
|
||||
{
|
||||
return a;
|
||||
}
|
||||
""".trimIndent().lines()
|
||||
|
||||
val symbols = extractSymbols(lines, SyntaxLanguage.CPP)
|
||||
|
||||
assertEquals(listOf("PreTranslateMessage(MSG*)", "After(int)"), symbols.map { it.label })
|
||||
assertEquals(9, symbols.first().endLine)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a forward declaration is not listed`() {
|
||||
val symbols = extractSymbols(listOf("class Foo;"), SyntaxLanguage.CPP)
|
||||
|
||||
Binary file not shown.
@ -1381,3 +1381,14 @@ This is the append-only implementation and handoff record shared by Codex and Cl
|
||||
- Live verification: installed on the SM-F966N. None of it has been seen on a screen by the
|
||||
implementer.
|
||||
- Next owner: the product owner, to confirm on the device.
|
||||
|
||||
## 2026-09-28 (continued) — TASK-058: the outline was empty for C++ written on Windows
|
||||
|
||||
- Request: the human product owner reported that a `.cpp` and `.h` showed no classes or functions.
|
||||
- Cause and change: see `tasks/058-outline-for-windows-cpp.md`. The main cause was `\r\n` line
|
||||
endings; the viewer kept the `\r` on every line, and no method pattern matched. This affected
|
||||
C# files with `\r\n` endings too.
|
||||
- Checks and results: the required checks pass; 435 unit tests, 0 failures; lint 0 errors.
|
||||
- Live verification: installed on the SM-F966N; reproduced and fixed in unit tests only, not on
|
||||
the owner's files.
|
||||
- Next owner: the product owner, to confirm on the files they were reading.
|
||||
|
||||
69
tasks/058-outline-for-windows-cpp.md
Normal file
69
tasks/058-outline-for-windows-cpp.md
Normal file
@ -0,0 +1,69 @@
|
||||
# TASK-058: The outline was empty for C++ written on Windows
|
||||
|
||||
Status: implemented; required checks pass; installed on the product owner's Z Fold7, awaiting their confirmation
|
||||
Owner: Claude
|
||||
Reviewer: human product owner
|
||||
|
||||
## Outcome
|
||||
|
||||
The human product owner reported that a `.cpp` and its `.h` showed no classes or functions in the
|
||||
side pane, although both plainly contain them.
|
||||
|
||||
## Cause
|
||||
|
||||
Reproduced in a unit test, not on the owner's files (their repository is private and was not read).
|
||||
|
||||
1. Lines ending in `\r\n`. The viewer split text on `\n` only, so every line of a file written on
|
||||
Windows ended in an invisible `\r`. The patterns that recognize a method must match a whole
|
||||
line, and `.` does not match `\r`, so no method was ever recognized -- in C++ and in C# alike.
|
||||
A class was still found, because that pattern only has to match the start of the line. A `.cpp`
|
||||
holding only function definitions therefore had an empty outline.
|
||||
2. Found while reproducing, in MFC-style code: a macro in a class body (`DECLARE_MESSAGE_MAP()`)
|
||||
was listed as a method, and because it has no `;` the declaration on the next line was skipped.
|
||||
3. Found while reproducing: a block opened in both branches of an `#if`/`#else` was counted twice,
|
||||
leaving a scope that never closed; every function after it in the file was missing.
|
||||
|
||||
## Scope
|
||||
|
||||
- Included: the three causes above, with a test for each.
|
||||
- Excluded: declarations produced by macros, and declarations split across lines before their
|
||||
name. The outline recognizes patterns; it is not a parser.
|
||||
|
||||
## Writable paths
|
||||
|
||||
- `app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/`
|
||||
- `app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/`
|
||||
- `tasks/`, `docs/`
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] A header, a source file and a C# file with `\r\n` line endings list their methods.
|
||||
- [x] A capitalized macro in a class body is not listed and does not hide what follows it.
|
||||
- [x] A block opened in each branch of a conditional is counted once.
|
||||
|
||||
## Security and privacy
|
||||
|
||||
No additional concerns.
|
||||
|
||||
## Required checks
|
||||
|
||||
```text
|
||||
./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug
|
||||
```
|
||||
|
||||
## Evidence
|
||||
|
||||
- Required checks pass: 435 unit tests, 0 failures; lint 0 errors.
|
||||
- Installed on the SM-F966N over wireless `adb`.
|
||||
- Not verified against the files the product owner was reading. If their outline is still empty,
|
||||
the cause is something these three do not cover, and a sample of the file is needed.
|
||||
|
||||
## Known limitations
|
||||
|
||||
- A header whose class showed nothing at all is not explained by cause 1, which still finds
|
||||
classes. It may be explained by the owner looking at the `.cpp`'s outline while both were open.
|
||||
- Removing the `\r` also changes what "copy" puts on the clipboard: lines are joined as displayed.
|
||||
|
||||
## Review findings
|
||||
|
||||
None yet.
|
||||
Loading…
x
Reference in New Issue
Block a user