Skip to content

Commit 9b169de

Browse files
mguzmanmmguzmanm
authored
Remove whole logic related to x / ! / ? merge symbols and add decoration N (#158)
* Remove whole logic related to x / ! / ? merge symbols and add decorator N * Improve merge UX: clarify tooltip and separate status from action * Fix lint issues
1 parent 515af88 commit 9b169de

9 files changed

Lines changed: 93 additions & 166 deletions

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@
649649
{
650650
"category": "CMSIS",
651651
"command": "cmsis-csolution.mergeFile",
652-
"title": "Merge Updated Config File",
652+
"title": "Open Merge View",
653653
"icon": "$(git-merge)"
654654
},
655655
{

‎src/views/solution-outline/tree-structure/solution-outline-file-item.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,12 @@ describe('FileItem', () => {
9595

9696
// check attributes and merge features for each file node
9797
const fileLabels = ['RTX_Config.c', 'RTX_Config.h'];
98-
const fileStatuses = ['- (X) \'RTX_Config.c\' is incompatible. A file update is mandatory.', '- (?) \'RTX_Config.h\' has corrections. A file update is suggested.'];
99-
const fileDescriptions = ['(X)', '(?)'];
10098
createdChildren.forEach((child, idx) => {
10199
expect(child.getTag()).toBe('file');
102100
expect(child.getAttribute('label')).toContain(fileLabels[idx]);
103101
expect(child.getAttribute('update')).toContain('update');
104102
expect(child.getAttribute('base')).toContain('base');
105-
expect(child.getAttribute('description')).toContain(fileDescriptions[idx]);
106-
expect(child.getAttribute('tooltip')).toContain(fileStatuses[idx]);
103+
expect(child.getAttribute('description')).toBeUndefined();
107104
});
108105

109106
});

‎src/views/solution-outline/tree-structure/solution-outline-file-item.ts‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import path from 'path';
1818
import { CTreeItem, ITreeItem } from '../../../generic/tree-item';
1919
import { FILE_TAGS } from '../../../solutions/constants';
2020
import { COutlineItem } from './solution-outline-item';
21-
import { getStatusTooltip, setContextMenuAttributes, setHeaderContext, setMergeDescription, setMergeFileContext } from './solution-outline-utils';
21+
import { setContextMenuAttributes, setHeaderContext, setMergeFileContext } from './solution-outline-utils';
2222
import { matchesContext } from '../../../utils/context-utils';
2323
import { SolutionOutlineItemBuilder } from './solution-outline-item-builder';
2424
import { CSolution } from '../../../solutions/csolution';
@@ -131,24 +131,6 @@ export class FileItemBuilder extends SolutionOutlineItemBuilder {
131131

132132
// assign merge context
133133
setMergeFileContext(cfileItem);
134-
135-
// assign description
136-
setMergeDescription(cfileItem, fileStatus);
137-
138-
// set tooltip
139-
const existingTooltip = cfileItem.getValue('tooltip');
140-
const label = cfileItem.getValue('label');
141-
142-
if (label) {
143-
const statusTooltip = getStatusTooltip(label, fileStatus);
144-
145-
if (existingTooltip) {
146-
cfileItem.setAttribute('tooltip', existingTooltip + '\n' + statusTooltip);
147-
} else {
148-
cfileItem.setAttribute('tooltip', statusTooltip);
149-
}
150-
}
151-
152134
cfileItem.setAttribute('local', localPath);
153135
cfileItem.setAttribute('update', updatePath);
154136
cfileItem.setAttribute('base', basePath);

‎src/views/solution-outline/tree-structure/solution-outline-hardware-item.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ describe('HardwareItemBuilder', () => {
107107
const gotBase = base ? path.basename(base) : '';
108108
const wantBase = 'Hello+CS300.dbgconf.base@0.0.1';
109109
expect(gotBase).toEqual(wantBase);
110-
111-
expect(device?.getAttribute('tooltip')).toContain('- (?) \'Hello+CS300.dbgconf\' has corrections. A file update is suggested.');
112110
});
113111

114112
});

‎src/views/solution-outline/tree-structure/solution-outline-project-items.ts‎

Lines changed: 2 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ import { FileItemBuilder } from './solution-outline-file-item';
2121
import { COutlineItem } from './solution-outline-item';
2222
import * as manifest from '../../../manifest';
2323
import { CSolution } from '../../../solutions/csolution';
24-
import { getMapFilePath, getStatusTooltip, setDocContext, setHeaderContext, setLinkerContext, setMergeDescription, setMergeUpdate } from './solution-outline-utils';
24+
import { getMapFilePath, setDocContext, setHeaderContext, setLinkerContext } from './solution-outline-utils';
2525
import { CProjectYamlFile } from '../../../solutions/files/cproject-yaml-file';
2626
import { SolutionOutlineItemBuilder } from './solution-outline-item-builder';
2727

2828
export class ProjectItemsBuilder extends SolutionOutlineItemBuilder {
29-
private _lastPrioritizedComponentList: COutlineItem[] = [];
29+
private readonly _lastPrioritizedComponentList: COutlineItem[] = [];
3030

3131
public get lastPrioritizedComponentList(): COutlineItem[] {
3232
return this._lastPrioritizedComponentList;
@@ -222,62 +222,9 @@ export class ProjectItemsBuilder extends SolutionOutlineItemBuilder {
222222
}
223223

224224
this.addComponentOptions(componentNodes);
225-
226-
// add merge description
227-
const components = Array.from(componentNodes.values());
228-
const fileStatus = this.getMergeDescriptionAtParentComponentLevel(components);
229-
if (fileStatus) {
230-
// assign description
231-
setMergeDescription(componentsItem, fileStatus);
232-
233-
// assign tooltip with component ids
234-
const prioritizedList = this._lastPrioritizedComponentList;
235-
let newTooltip = 'Components with updated configuration files:';
236-
for (const comp of prioritizedList) {
237-
const compId = comp.getAttribute('label');
238-
const compStatus = comp.getAttribute('status');
239-
if (compId && compStatus) {
240-
const update = comp.getAttribute('update');
241-
newTooltip += `\n- ${update} ${compId}: ${compStatus}`;
242-
}
243-
}
244-
componentsItem.setAttribute('tooltip', newTooltip);
245-
}
246225
return true; // do have components to edit
247226
}
248227

249-
private getMergeDescriptionAtParentComponentLevel(components: COutlineItem[]): string | undefined {
250-
let result: string | undefined = undefined;
251-
const updateRequired: COutlineItem[] = [];
252-
const updateRecommended: COutlineItem[] = [];
253-
const updateSuggested: COutlineItem[] = [];
254-
255-
for (const component of components) {
256-
const status = component.getAttribute('status');
257-
if (!status) {
258-
continue;
259-
}
260-
if (status == 'update required') {
261-
updateRequired.push(component);
262-
} else if (status == 'update recommended') {
263-
updateRecommended.push(component);
264-
} else if (status == 'update suggested') {
265-
updateSuggested.push(component);
266-
}
267-
}
268-
269-
const prioritizedList = [...updateRequired, ...updateRecommended, ...updateSuggested];
270-
this._lastPrioritizedComponentList = prioritizedList;
271-
const prioritizedFile = prioritizedList[0];
272-
273-
const fileStatus = prioritizedFile?.getAttribute('status');
274-
if (fileStatus) {
275-
result = fileStatus;
276-
}
277-
278-
return result;
279-
}
280-
281228
private addComponentOptions(componentNodes: Map<string, COutlineItem>) {
282229
const components = componentNodes.values();
283230

@@ -437,31 +384,6 @@ export class ProjectItemsBuilder extends SolutionOutlineItemBuilder {
437384

438385
// set status at component level
439386
node.setAttribute('status', fileStatus);
440-
441-
// assign description
442-
setMergeDescription(node, fileStatus);
443-
444-
// assign status symbol for merge update action
445-
setMergeUpdate(node, fileStatus);
446-
447-
// set tooltip
448-
const prevTooltip = node.getValue('tooltip');
449-
let newTooltip: string = '';
450-
for (const file of prioritizedList) {
451-
const fileLabel = file.getValue('file');
452-
const fileStatus = file.getValue('status');
453-
if (fileLabel && fileStatus) {
454-
const tooltip = getStatusTooltip(fileLabel, fileStatus);
455-
newTooltip += `\n ${tooltip}`;
456-
}
457-
}
458-
459-
if (prevTooltip) {
460-
node.setAttribute('tooltip', prevTooltip + '\n' + newTooltip);
461-
} else {
462-
node.setAttribute('tooltip', newTooltip);
463-
}
464-
465387
}
466388

467389
private getPrioritizedMergeFile(files: ITreeItem<CTreeItem>[]): ITreeItem<CTreeItem>[] {

‎src/views/solution-outline/tree-structure/solution-outline-tree.ts‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import * as manifest from '../../../manifest';
2323
import { HardwareItemBuilder } from './solution-outline-hardware-item';
2424
import { ProjectItemsBuilder } from './solution-outline-project-items';
2525
import { getFileNameNoExt } from '../../../utils/path-utils';
26-
import { setMergeDescription } from './solution-outline-utils';
2726
import { CProjectYamlFile } from '../../../solutions/files/cproject-yaml-file';
2827
import { SolutionOutlineItemBuilder } from './solution-outline-item-builder';
2928

@@ -122,26 +121,6 @@ export class SolutionOutlineTree extends SolutionOutlineItemBuilder {
122121
if (cproject) {
123122
const projectItems = new ProjectItemsBuilder(this.csolution, this.rpcData, context);
124123
projectItems.addProjectChildren(this.csolution, cprojectItem, cprojectFile, cbuild);
125-
126-
// get prioritized component list and set merge description if available
127-
const prioritizedList = projectItems.lastPrioritizedComponentList;
128-
if (prioritizedList && prioritizedList.length > 0) {
129-
const fileStatus = prioritizedList[0].getAttribute('status');
130-
if (fileStatus) {
131-
setMergeDescription(cprojectItem, fileStatus);
132-
133-
// set tooltip
134-
const existingTooltip = cprojectItem.getAttribute('tooltip');
135-
const description = cprojectItem.getAttribute('description');
136-
const newTooltip = `- ${description} Component config files: ${fileStatus}`;
137-
138-
if (existingTooltip) {
139-
cprojectItem.setAttribute('tooltip', existingTooltip + '\n' + newTooltip);
140-
} else {
141-
cprojectItem.setAttribute('tooltip', newTooltip);
142-
}
143-
}
144-
}
145124
} else {
146125
cprojectItem.setAttribute('description', 'error loading project');
147126
}

‎src/views/solution-outline/tree-structure/solution-outline-utils.ts‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,6 @@ export function setMergeFiles(component: COutlineItem, file: ITreeItem<CTreeItem
5252
component.setAttribute('base', basePath);
5353
}
5454

55-
export function getStatusTooltip(label: string, status: string): string | undefined {
56-
switch (status) {
57-
case 'update suggested':
58-
return `- (?) '${label}' has corrections. A file update is suggested.`;
59-
case 'update recommended':
60-
return `- (!) '${label}' has new features. A file update is recommended.`;
61-
case 'update required':
62-
return `- (X) '${label}' is incompatible. A file update is mandatory.`;
63-
default:
64-
return undefined;
65-
}
66-
}
67-
6855
export function setLinkerContext(node: COutlineItem, mapFilePath: string): void {
6956
node.addFeature(`${manifest.LINKER_CONTEXT}`);
7057
node.setAttribute('type', 'linkerMapFile');
@@ -83,20 +70,6 @@ export function getMapFilePath(cbuild: CTreeItem): string | undefined {
8370
return mapFile ? path.join(outDirPath, mapFile) : findFirstMapFile(outDirPath);
8471
}
8572

86-
export function setMergeDescription(node: COutlineItem, fileStatus: string): void {
87-
const desc = getMergeStatusSymbol(fileStatus);
88-
if (desc !== undefined) {
89-
node.setAttribute('description', desc);
90-
}
91-
}
92-
93-
export function setMergeUpdate(node: COutlineItem, fileStatus: string): void {
94-
const update = getMergeStatusSymbol(fileStatus);
95-
if (update !== undefined) {
96-
node.setAttribute('update', update);
97-
}
98-
}
99-
10073
function getOutdirPath(cbuild: CTreeItem): string | undefined {
10174
const outputDirs = cbuild.getChild('output-dirs');
10275
const outdir = outputDirs?.getValue('outdir');
@@ -128,15 +101,3 @@ function findFirstMapFile(dirPath: string): string | undefined {
128101
return undefined;
129102
}
130103

131-
function getMergeStatusSymbol(fileStatus: string): string | undefined {
132-
switch (fileStatus) {
133-
case 'update required':
134-
return '(X)';
135-
case 'update recommended':
136-
return '(!)';
137-
case 'update suggested':
138-
return '(?)';
139-
default:
140-
return undefined;
141-
}
142-
}

‎src/views/solution-outline/treeview-decoration-provider.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,77 @@ describe('provideFileDecoration', () => {
182182

183183
expect(refreshSpy).toHaveBeenCalled();
184184
});
185+
186+
it('should return merge decoration for files with merge feature', () => {
187+
const filePath = path.join('src', 'merge', 'file.c');
188+
const uri = URI.file(filePath);
189+
190+
const root = new COutlineItem('root');
191+
const project = root.createChild('project');
192+
const group = project.createChild('group');
193+
const file = group.createChild('file');
194+
file.setTag('file');
195+
file.setAttribute('resourcePath', uri.fsPath);
196+
file.addFeature('mergeFile:');
197+
198+
treeViewDecorationProvider.setTreeRoot(root);
199+
200+
const result = treeViewDecorationProvider.provideFileDecoration(uri);
201+
202+
expect(result).toEqual({
203+
badge: TreeViewFileDecorationProvider.mergeBadge,
204+
tooltip: TreeViewFileDecorationProvider.mergeTooltip,
205+
color: { id: TreeViewFileDecorationProvider.mergeColor },
206+
});
207+
});
208+
209+
it('should prioritize excluded decoration over merge decoration', () => {
210+
const filePath = path.join('src', 'merge', 'excluded.c');
211+
const uri = URI.file(filePath);
212+
213+
const root = new COutlineItem('root');
214+
const project = root.createChild('project');
215+
const group = project.createChild('group');
216+
const file = group.createChild('file');
217+
file.setTag('file');
218+
file.setAttribute('resourcePath', uri.fsPath);
219+
file.addFeature('mergeFile:');
220+
file.setAttribute('excluded', '1');
221+
222+
treeViewDecorationProvider.setTreeRoot(root);
223+
224+
const result = treeViewDecorationProvider.provideFileDecoration(uri);
225+
226+
expect(result).toEqual({
227+
badge: TreeViewFileDecorationProvider.excludedBadge,
228+
tooltip: TreeViewFileDecorationProvider.excludedTooltip,
229+
color: { id: TreeViewFileDecorationProvider.excludedColor },
230+
});
231+
});
232+
233+
it('should prioritize merge decoration over pack-sourced decoration', () => {
234+
const packCachePath = 'MOCKED_CMSIS_PACK_ROOT';
235+
jest.spyOn(path_utils, 'getCmsisPackRoot').mockReturnValue(packCachePath);
236+
237+
const filePath = path.join(packCachePath, 'root', 'merge.c');
238+
const uri = URI.file(filePath);
239+
240+
const root = new COutlineItem('root');
241+
const project = root.createChild('project');
242+
const group = project.createChild('group');
243+
const file = group.createChild('file');
244+
file.setTag('file');
245+
file.setAttribute('resourcePath', uri.fsPath);
246+
file.addFeature('mergeFile:');
247+
248+
treeViewDecorationProvider.setTreeRoot(root);
249+
250+
const result = treeViewDecorationProvider.provideFileDecoration(uri);
251+
252+
expect(result).toEqual({
253+
badge: TreeViewFileDecorationProvider.mergeBadge,
254+
tooltip: TreeViewFileDecorationProvider.mergeTooltip,
255+
color: { id: TreeViewFileDecorationProvider.mergeColor },
256+
});
257+
});
185258
});

‎src/views/solution-outline/treeview-decoration-provider.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,17 @@ import { ThemeProvider } from '../../vscode-api/theme-provider';
2020
import { URI } from 'vscode-uri';
2121
import { getCmsisPackRoot } from '../../utils/path-utils';
2222
import { COutlineItem } from './tree-structure/solution-outline-item';
23+
import * as manifest from '../../manifest';
2324

2425
export class TreeViewFileDecorationProvider implements vscode.FileDecorationProvider {
2526
static readonly badge: string = 'P';
2627
static readonly tooltip: string = 'Pack sourced';
2728
static readonly themeColor: string = 'descriptionForeground';
2829

30+
static readonly mergeBadge: string = 'N';
31+
static readonly mergeTooltip: string = 'New Version Available for Merge';
32+
static readonly mergeColor: string = 'charts.blue';
33+
2934
static readonly excludedBadge: string = 'X';
3035
static readonly excludedTooltip: string = 'Excluded from build';
3136
static readonly excludedColor: string = 'errorForeground';
@@ -60,6 +65,16 @@ export class TreeViewFileDecorationProvider implements vscode.FileDecorationProv
6065
};
6166
}
6267

68+
// Merge-enabled files have higher priority than pack-sourced files
69+
const features = fileItem?.getFeatures().split(';') ?? [];
70+
if (features.includes(manifest.MERGE_FILE_CONTEXT)) {
71+
return {
72+
badge: TreeViewFileDecorationProvider.mergeBadge,
73+
tooltip: TreeViewFileDecorationProvider.mergeTooltip,
74+
color: this.themeProvider.getThemeColor(TreeViewFileDecorationProvider.mergeColor),
75+
};
76+
}
77+
6378
// Check for pack-sourced files
6479
const cmsisPackRoot = getCmsisPackRoot();
6580
const cmsisPackRootUri = URI.file(cmsisPackRoot);

0 commit comments

Comments
 (0)