feat: implement change list sync with JetBrains/PhpStorm and bug fixes - #4
Open
radoslavius wants to merge 3 commits into
Open
feat: implement change list sync with JetBrains/PhpStorm and bug fixes#4radoslavius wants to merge 3 commits into
radoslavius wants to merge 3 commits into
Conversation
radoslavius
commented
Jun 22, 2026
radoslavius
commented
- Add bi-directional change list sync with JetBrains/PhpStorm (using file watcher with periodic polling fallback for WSL/UNC paths)
- Fix Git commit tracking to properly remove committed files and prevent files from "bouncing" when moved back to Default list
- Fix path casing and slashes normalization for seamless compatibility in Windows/WSL environments
- Fix initialization race condition where imported change list mappings were wiped prematurely
- Add support for JetBrains
$PROJECT_DIR$ macro in workspace.xml paths - Add active change list indicator to the Status Bar with a quick switch menu
- Force virtual "Unversioned Files" list to always be sorted at the end of the view
- Sync package-lock.json to align with the AGPL-3.0-only license
- Add bi-directional change list sync with JetBrains/PhpStorm (using file watcher with periodic polling fallback for WSL/UNC paths) - Fix Git commit tracking to properly remove committed files and prevent files from "bouncing" when moved back to Default list - Fix path casing and slashes normalization for seamless compatibility in Windows/WSL environments - Fix initialization race condition where imported change list mappings were wiped prematurely - Add support for JetBrains $PROJECT_DIR$ macro in workspace.xml paths - Add active change list indicator to the Status Bar with a quick switch menu - Force virtual "Unversioned Files" list to always be sorted at the end of the view - Sync package-lock.json to align with the AGPL-3.0-only license
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces bi-directional synchronization of change lists with JetBrains IDEs (PhpStorm/IntelliJ) via the ".idea/workspace.xml" file, adding the "IdeaSyncService", status bar integration, and corresponding configuration settings. The review feedback highlights several critical and high-severity issues in the synchronization service, including the lack of XML escaping when writing to "workspace.xml", missing unescaping of XML entities when importing, fragile manual path concatenation instead of using "path.join", and potential race conditions in the file watcher due to uncleared timeouts. Additionally, it is recommended to avoid using "any" casts in command node handling to maintain TypeScript type safety.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+250
to
+271
| private async generateXmlBlock(): Promise<string> { | ||
| const lists = this.changeListManager.getLists().filter(l => !l.isReadOnly); | ||
| const xmlLines: string[] = []; | ||
|
|
||
| xmlLines.push(' <component name="ChangeListManager">'); | ||
|
|
||
| for (const list of lists) { | ||
| const defaultAttr = list.isDefault ? ' default="true"' : ''; | ||
| const files = await this.changeListManager.getFilesForList(list.id); | ||
|
|
||
| if (files.length === 0) { | ||
| xmlLines.push(` <list${defaultAttr} id="${list.id}" name="${list.name}" comment="" />`); | ||
| } else { | ||
| xmlLines.push(` <list${defaultAttr} id="${list.id}" name="${list.name}" comment="">`); | ||
| for (const file of files) { | ||
| // JetBrains uses forward slashes in relative paths | ||
| const relPath = file.relativePath.replace(/\\/g, '/'); | ||
| xmlLines.push(` <change beforePath="$PROJECT_DIR$/${relPath}" beforeDir="false" afterPath="$PROJECT_DIR$/${relPath}" afterDir="false" />`); | ||
| } | ||
| xmlLines.push(' </list>'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Writing raw strings (such as change list names, IDs, or file paths) directly into XML attributes without escaping can corrupt the .idea/workspace.xml file if they contain special characters like &, <, >, ", or '. For example, a change list named Features & Bugs will produce invalid XML and corrupt the configuration file. Please escape these values before writing them.
private async generateXmlBlock(): Promise<string> {
const lists = this.changeListManager.getLists().filter(l => !l.isReadOnly);
const xmlLines: string[] = [];
const escapeXml = (unsafe: string) => unsafe.replace(/[<>&'"]/g, (c) => {
switch (c) {
case '<': return '<';
case '>': return '>';
case '&': return '&';
case '\'': return ''';
case '"': return '"';
default: return c;
}
});
xmlLines.push(' <component name="ChangeListManager">');
for (const list of lists) {
const defaultAttr = list.isDefault ? ' default="true"' : '';
const files = await this.changeListManager.getFilesForList(list.id);
const escapedId = escapeXml(list.id);
const escapedName = escapeXml(list.name);
if (files.length === 0) {
xmlLines.push(` <list${defaultAttr} id="${escapedId}" name="${escapedName}" comment="" />`);
} else {
xmlLines.push(` <list${defaultAttr} id="${escapedId}" name="${escapedName}" comment="">`);
for (const file of files) {
// JetBrains uses forward slashes in relative paths
const relPath = escapeXml(file.relativePath.replace(/\\/g, '/'));
xmlLines.push(` <change beforePath="$PROJECT_DIR$/${relPath}" beforeDir="false" afterPath="$PROJECT_DIR$/${relPath}" afterDir="false" />`);
}
xmlLines.push(' </list>');
}
}
Comment on lines
+145
to
+147
| const id = idMatch[1]; | ||
| const name = nameMatch[1]; | ||
| const isDefault = defaultMatch ? defaultMatch[1] === 'true' : false; |
There was a problem hiding this comment.
When importing from XML, attributes like name might contain escaped XML entities (e.g., &). If not unescaped, they will display as raw entities in VS Code. Please unescape these values when parsing them.
Suggested change
| const id = idMatch[1]; | |
| const name = nameMatch[1]; | |
| const isDefault = defaultMatch ? defaultMatch[1] === 'true' : false; | |
| const id = idMatch[1]; | |
| const name = nameMatch[1] | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"') | |
| .replace(/'/g, "'"); | |
| const isDefault = defaultMatch ? defaultMatch[1] === 'true' : false; |
Comment on lines
+163
to
+169
| const relativePath = pathMatch[1]; | ||
| // Combine workspace root and relative path in a platform-independent way | ||
| const normalizedRoot = workspaceRoot.replace(/\\/g, '/'); | ||
| const absolutePath = normalizedRoot.endsWith('/') | ||
| ? normalizedRoot + relativePath | ||
| : normalizedRoot + '/' + relativePath; | ||
| fileMapping[normalizePathKey(absolutePath)] = id; |
There was a problem hiding this comment.
Manual string concatenation and slash replacement for combining workspaceRoot and relativePath is fragile and error-prone. Using path.join is platform-independent, cleaner, and automatically handles path separators correctly.
Suggested change
| const relativePath = pathMatch[1]; | |
| // Combine workspace root and relative path in a platform-independent way | |
| const normalizedRoot = workspaceRoot.replace(/\\/g, '/'); | |
| const absolutePath = normalizedRoot.endsWith('/') | |
| ? normalizedRoot + relativePath | |
| : normalizedRoot + '/' + relativePath; | |
| fileMapping[normalizePathKey(absolutePath)] = id; | |
| const relativePath = pathMatch[1]; | |
| const absolutePath = path.join(workspaceRoot, relativePath); | |
| fileMapping[normalizePathKey(absolutePath)] = id; |
Comment on lines
+64
to
+82
| this.disposables.push( | ||
| this.fileWatcher.onDidChange(async () => { | ||
| if (this.isWriting) { | ||
| logger.info('IdeaSyncService: Ignored file change event (internal write)'); | ||
| return; | ||
| } | ||
| logger.info('IdeaSyncService: External change to workspace.xml detected via watcher, importing in 100ms...'); | ||
| setTimeout(async () => { | ||
| await this.importFromIdea(); | ||
| }, 100); | ||
| }), | ||
| this.fileWatcher.onDidCreate(async () => { | ||
| if (this.isWriting) return; | ||
| logger.info('IdeaSyncService: workspace.xml created via watcher, importing in 100ms...'); | ||
| setTimeout(async () => { | ||
| await this.importFromIdea(); | ||
| }, 100); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
The file watcher triggers imports using setTimeout without clearing previous timeouts. If multiple file changes occur in rapid succession, multiple imports will run concurrently, causing race conditions and redundant disk reads. Storing and clearing the timeout properly debounces the import.
let readTimeout: NodeJS.Timeout | undefined;
this.disposables.push(
this.fileWatcher.onDidChange(async () => {
if (this.isWriting) {
logger.info('IdeaSyncService: Ignored file change event (internal write)');
return;
}
logger.info('IdeaSyncService: External change to workspace.xml detected via watcher, scheduling import...');
if (readTimeout) {
clearTimeout(readTimeout);
}
readTimeout = setTimeout(async () => {
await this.importFromIdea();
}, 100);
}),
this.fileWatcher.onDidCreate(async () => {
if (this.isWriting) return;
logger.info('IdeaSyncService: workspace.xml created via watcher, scheduling import...');
if (readTimeout) {
clearTimeout(readTimeout);
}
readTimeout = setTimeout(async () => {
await this.importFromIdea();
}, 100);
})
);
Comment on lines
232
to
249
| .map((n: any) => { | ||
| if (n && n.type === 'file' && n.change && n.change.resourceUri) { | ||
| return n.change.resourceUri.fsPath; | ||
| } | ||
| if (n && n.resourceUri) { | ||
| return n.resourceUri.fsPath; | ||
| } | ||
| return ''; | ||
| }) | ||
| .filter(Boolean); | ||
| } else if (node) { | ||
| const anyNode = node as any; | ||
| if (anyNode.type === 'file' && anyNode.change && anyNode.change.resourceUri) { | ||
| filesToMove = [anyNode.change.resourceUri.fsPath]; | ||
| } else if (anyNode.resourceUri) { | ||
| filesToMove = [anyNode.resourceUri.fsPath]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Casting to any bypasses TypeScript's type safety. Defining a proper shape or using a safe type assertion prevents potential runtime errors and maintains type safety.
.map((n) => {
const item = n as { type?: string; change?: { resourceUri?: vscode.Uri }; resourceUri?: vscode.Uri };
if (item && item.type === 'file' && item.change?.resourceUri) {
return item.change.resourceUri.fsPath;
}
if (item && item.resourceUri) {
return item.resourceUri.fsPath;
}
return '';
})
.filter(Boolean);
} else if (node) {
const item = node as { type?: string; change?: { resourceUri?: vscode.Uri }; resourceUri?: vscode.Uri };
if (item.type === 'file' && item.change?.resourceUri) {
filesToMove = [item.change.resourceUri.fsPath];
} else if (item.resourceUri) {
filesToMove = [item.resourceUri.fsPath];
}There was a problem hiding this comment.
Pull request overview
Implements JetBrains/PhpStorm change list synchronization and improves cross-platform change-list behavior (notably Windows/WSL path normalization and Git staging/commit tracking), plus UI integration via a status bar indicator and SCM context menu entry.
Changes:
- Add bi-directional PhpStorm
.idea/workspace.xmlsync service with watcher + polling fallback and new settings (ideaSync.enabled,ideaSync.interval). - Normalize file-path keys and adjust change list sorting (keep “Unversioned Files” last).
- Fix Git commit tracking/staging behavior and add status bar + SCM context menu integration.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/helpers.ts | Adds normalized map key helper + improves relative path detection + forces Unversioned list to sort last. |
| src/utils/constants.ts | Adds new configuration keys for JetBrains sync. |
| src/services/ideaSyncService.ts | New service to import/export change lists to .idea/workspace.xml with watcher + polling. |
| src/services/gitService.ts | Improves commit detection ordering; stages/unstages via git with path normalization and “programmatic operation” tracking. |
| src/services/configService.ts | Exposes new Idea sync settings accessors. |
| src/services/changeListManager.ts | Uses normalized path keys for mappings; adds import from JetBrains; adjusts stale-mapping cleanup usage. |
| src/extension.ts | Wires Idea sync service; adds status bar indicator for active change list. |
| src/commands/index.ts | Extends “move to list” command handling to support SCM resource context nodes. |
| README.md | Documents JetBrains sync feature and settings. |
| package.json | Adds SCM context menu entry; contributes Idea sync configuration settings. |
| package-lock.json | Updates package-lock license metadata. |
| docs/CONFIGURATION.md | Adds Idea sync settings documentation rows (but namespace consistency needs attention). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+355
to
+359
| // Track these as programmatic operations to prevent auto-assignment bounce-back | ||
| for (const filePath of filePaths) { | ||
| this.programmaticOperations.add(filePath); | ||
| } | ||
|
|
Comment on lines
+7
to
+8
| import { Logger, logger } from '../utils/logger'; | ||
| import { normalizePathKey } from '../utils/helpers'; |
Comment on lines
+250
to
+271
| private async generateXmlBlock(): Promise<string> { | ||
| const lists = this.changeListManager.getLists().filter(l => !l.isReadOnly); | ||
| const xmlLines: string[] = []; | ||
|
|
||
| xmlLines.push(' <component name="ChangeListManager">'); | ||
|
|
||
| for (const list of lists) { | ||
| const defaultAttr = list.isDefault ? ' default="true"' : ''; | ||
| const files = await this.changeListManager.getFilesForList(list.id); | ||
|
|
||
| if (files.length === 0) { | ||
| xmlLines.push(` <list${defaultAttr} id="${list.id}" name="${list.name}" comment="" />`); | ||
| } else { | ||
| xmlLines.push(` <list${defaultAttr} id="${list.id}" name="${list.name}" comment="">`); | ||
| for (const file of files) { | ||
| // JetBrains uses forward slashes in relative paths | ||
| const relPath = file.relativePath.replace(/\\/g, '/'); | ||
| xmlLines.push(` <change beforePath="$PROJECT_DIR$/${relPath}" beforeDir="false" afterPath="$PROJECT_DIR$/${relPath}" afterDir="false" />`); | ||
| } | ||
| xmlLines.push(' </list>'); | ||
| } | ||
| } |
Comment on lines
+65
to
+73
| this.fileWatcher.onDidChange(async () => { | ||
| if (this.isWriting) { | ||
| logger.info('IdeaSyncService: Ignored file change event (internal write)'); | ||
| return; | ||
| } | ||
| logger.info('IdeaSyncService: External change to workspace.xml detected via watcher, importing in 100ms...'); | ||
| setTimeout(async () => { | ||
| await this.importFromIdea(); | ||
| }, 100); |
Comment on lines
+49
to
+53
| * Normalize path for use as a map key (lowercase, forward slashes) | ||
| */ | ||
| export function normalizePathKey(filePath: string): string { | ||
| return filePath.replace(/\\/g, '/').toLowerCase(); | ||
| } |
Comment on lines
231
to
+241
| filesToMove = nodes | ||
| .filter((n): n is FileNode => n.type === 'file') | ||
| .map((n) => n.change.resourceUri.fsPath); | ||
| } else if (node && node.type === 'file') { | ||
| filesToMove = [node.change.resourceUri.fsPath]; | ||
| .map((n: any) => { | ||
| if (n && n.type === 'file' && n.change && n.change.resourceUri) { | ||
| return n.change.resourceUri.fsPath; | ||
| } | ||
| if (n && n.resourceUri) { | ||
| return n.resourceUri.fsPath; | ||
| } | ||
| return ''; | ||
| }) | ||
| .filter(Boolean); |
Comment on lines
56
to
65
| | `smartCommit.defaultViewMode` | `"list"` \| `"tree"` | `"list"` | Workspace | Default view mode for file display | | ||
| | `smartCommit.showStatusBar` | `boolean` | `true` | Global | Show active list in status bar | | ||
| | `smartCommit.confirmDeleteNonEmpty` | `boolean` | `true` | Global | Confirm before deleting non-empty lists | | ||
| | `smartCommit.autoActivateNew` | `boolean` | `true` | Global | Auto-activate newly created lists | | ||
| | `smartCommit.commitGuard.enabled` | `boolean` | `true` | Global | Warn when staging mixed change lists | | ||
| | `smartCommit.commitGuard.interceptCommit` | `boolean` | `false` | Global | Intercept native commit command | | ||
| | `smartCommit.autoAssignStagedFiles` | `boolean` | `true` | Global | Auto-assign externally staged files | | ||
| | `gitChangeLists.ideaSync.enabled` | `boolean` | `true` | Workspace | Enable PhpStorm bi-directional synchronization | | ||
| | `gitChangeLists.ideaSync.interval` | `integer` | `1000` | Global | Debounce interval (in ms) for PhpStorm sync | | ||
| | `smartCommit.debug.enableLogging` | `boolean` | `true` | Global | Enable verbose debug logging | |
…iles in build artifacts
- Add XML escaping and unescaping when exporting/importing change lists in IdeaSyncService. - Replace manual path concatenation with robust `path.join` calls. - Fix file watcher timeout leak by debouncing and clearing `readTimeout` on dispose. - Remove `any` casts in `MOVE_TO_LIST` command in favor of safe type assertions. - Add changelog entry for today's fixes.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.