Fixing compilation errors after platform localization. Some refactoring

[unv-unv]

No description provided.

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 1m 1s —— View job

---

PR #2 review

• Read the diff
• Review the changes
• Post inline comments

Summary: The compilation fix looks correct. The changes to getPresentableText() (now returns LocalizeValue), @RequiredReadAction, and pattern-matching instanceof all look right. I didn't build the project; this review is from reading the diff only. All four inline comments are minor:

1. Line 75: removing @Nullable left two blank lines (one-click suggestion).
2. Line 65: getAlphaSortKey() returns null, but its contract expects non-null. This existed before the PR and is harmless while there are no sorters. A suggestion with a non-null fallback is attached.
3. Line 22: the file now mixes jspecify @Nullable with JetBrains @NotNull. I suggest using jspecify throughout, with a @NullMarked package.
4. Line 155: overrides that never return null don't need @Nullable. Also, "Header (%s entries)" is a hard-coded user-visible string that could be moved to LocalizeValue.

[github-actions]

Inline review: the compilation fix and the switch to pattern-matching instanceof look correct. The remaining notes are minor (style, nullability consistency, localization).

[github-actions]

Nit: removing @Nullable left two blank lines here.

Suggested change

[github-actions]

This existed before the PR: CSV fields aren't PsiNamedElement, so this always returns null, but SortableTreeElement#getAlphaSortKey is expected to be non-null. It's harmless today because CsvStructureViewModel.getSorters() returns Sorter.EMPTY_ARRAY, but adding Sorter.ALPHA later would cause an NPE. A non-null fallback avoids that:

Suggested change

	return myElement instanceof PsiNamedElement namedElem ? namedElem.getName() : null;
	return myElement instanceof PsiNamedElement namedElem && namedElem.getName() != null ? namedElem.getName() : "";

[github-actions]

This switches to jspecify @Nullable but keeps org.jetbrains.annotations.NotNull (line 21, used on Header.getChildren() and Field.getChildren()), so the file now mixes two nullability systems. To use jspecify consistently, mark the package @NullMarked (as editor/table/package-info.java already does) and then drop the JetBrains @NotNull annotations and import.

[github-actions]

Minor: Header.getLocationString(), Header.getIcon() and Field.getLocationString() never return null, so the overrides could drop @Nullable (a non-null return can narrow a nullable one).

Also, "Header (%s entries)" is a user-visible hard-coded string. Since this PR follows the platform localization change, consider moving it to the plugin's localize YAML and a LocalizeValue, either here or in a follow-up.

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 40s —— View job

---

PR #2 review (second pass)

• Read the diff (origin/master...18af998)
• Compare it with the inline threads from the first review
• Look for new issues

I re-reviewed the current head, 18af998. I didn't add new inline comments because I found no new issues, and reposting the open threads would only create duplicates.

Status of the first review's threads:

Thread	Status
Two blank lines left after removing @Nullable (around line 75)	✅ Fixed in the current head. The thread is outdated and can be resolved.
Line 65: getAlphaSortKey() can return null	Still open. This existed before the PR. It's harmless while getSorters() returns Sorter.EMPTY_ARRAY, but it would cause an NPE if Sorter.ALPHA is added later. The thread has a one-click suggestion.
Line 22: the file uses both jspecify @Nullable and JetBrains @NotNull	Still open. Making the package @NullMarked and dropping org.jetbrains.annotations.NotNull (lines 138 and 171) would make it consistent.
Line 154: @Nullable on overrides that never return null, and the hard-coded "Header (%s entries)"	Still open. Minor, could be a follow-up.

Checked this pass, no problems found:

• getPresentableText() returning LocalizeValue.of(myElement.getText()) is the right choice for raw PSI text: it's data, not a translatable string. Empty fields from CsvHelper.createEmptyCsvField have "" as their text, not null, so of(...) is safe.
• The @RequiredReadAction annotations on navigate, canNavigate*, getAlphaSortKey, getPresentableText and getIcon match where PSI is accessed.
• The pattern-matching instanceof rewrites (lines 45, 53, 59, 65, 70 and 103) keep the original behaviour.

Verdict: the compilation fix looks correct and safe to merge. The three open threads are optional cleanups. As before, I didn't build the project; this review is from reading the code only.