Skip to content

Commit c645a33

Browse files
committed
1 parent ff98f0f commit c645a33

2 files changed

Lines changed: 129 additions & 21 deletions

File tree

src/vs/platform/userDataSync/common/settingsMerge.ts

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -429,26 +429,34 @@ function getEditToInsertAtLocation(content: string, key: string, value: any, loc
429429

430430
if (location.insertAfter) {
431431

432+
const edits: Edit[] = [];
433+
432434
/* Insert after a setting */
433435
if (node.setting) {
434-
return [{ offset: node.endOffset, length: 0, content: ',' + newProperty }];
436+
edits.push({ offset: node.endOffset, length: 0, content: ',' + newProperty });
435437
}
436438

437-
/*
438-
Insert after a comment and before a setting (or)
439-
Insert between comments and there is a setting after
440-
*/
441-
if (tree[location.index + 1] &&
442-
(tree[location.index + 1].setting || findNextSettingNode(location.index, tree))) {
443-
return [{ offset: node.endOffset, length: 0, content: eol + newProperty + ',' }];
444-
}
439+
/* Insert after a comment */
440+
else {
441+
442+
const nextSettingNode = findNextSettingNode(location.index, tree);
443+
const previousSettingNode = findPreviousSettingNode(location.index, tree);
444+
const previousSettingCommaOffset = previousSettingNode?.setting?.commaOffset;
445445

446-
/* Insert after the comment at the end */
447-
const edits = [{ offset: node.endOffset, length: 0, content: eol + newProperty }];
448-
const previousSettingNode = findPreviousSettingNode(location.index, tree);
449-
if (previousSettingNode && !previousSettingNode.setting!.hasCommaSeparator) {
450-
edits.splice(0, 0, { offset: previousSettingNode.endOffset, length: 0, content: ',' });
446+
/* If there is a previous setting and it does not has comma then add it */
447+
if (previousSettingNode && previousSettingCommaOffset === undefined) {
448+
edits.push({ offset: previousSettingNode.endOffset, length: 0, content: ',' });
449+
}
450+
451+
const isPreviouisSettingIncludesComment = previousSettingCommaOffset !== undefined && previousSettingCommaOffset > node.endOffset;
452+
edits.push({
453+
offset: isPreviouisSettingIncludesComment ? previousSettingCommaOffset! + 1 : node.endOffset,
454+
length: 0,
455+
content: nextSettingNode ? eol + newProperty + ',' : eol + newProperty
456+
});
451457
}
458+
459+
452460
return edits;
453461
}
454462

@@ -516,7 +524,7 @@ interface INode {
516524
readonly value: string;
517525
readonly setting?: {
518526
readonly key: string;
519-
readonly hasCommaSeparator: boolean;
527+
readonly commaOffset: number | undefined;
520528
};
521529
readonly comment?: string;
522530
}
@@ -547,7 +555,7 @@ function parseSettings(content: string): INode[] {
547555
value: content.substring(startOffset, offset + length),
548556
setting: {
549557
key,
550-
hasCommaSeparator: false
558+
commaOffset: undefined
551559
}
552560
});
553561
}
@@ -564,7 +572,7 @@ function parseSettings(content: string): INode[] {
564572
value: content.substring(startOffset, offset + length),
565573
setting: {
566574
key,
567-
hasCommaSeparator: false
575+
commaOffset: undefined
568576
}
569577
});
570578
}
@@ -577,23 +585,29 @@ function parseSettings(content: string): INode[] {
577585
value: content.substring(startOffset, offset + length),
578586
setting: {
579587
key,
580-
hasCommaSeparator: false
588+
commaOffset: undefined
581589
}
582590
});
583591
}
584592
},
585593
onSeparator: (sep: string, offset: number, length: number) => {
586594
if (hierarchyLevel === 0) {
587595
if (sep === ',') {
588-
const node = nodes.pop();
596+
let index = nodes.length - 1;
597+
for (; index >= 0; index--) {
598+
if (nodes[index].setting) {
599+
break;
600+
}
601+
}
602+
const node = nodes[index];
589603
if (node) {
590-
nodes.push({
604+
nodes.splice(index, 1, {
591605
startOffset: node.startOffset,
592606
endOffset: node.endOffset,
593607
value: node.value,
594608
setting: {
595609
key: node.setting!.key,
596-
hasCommaSeparator: true
610+
commaOffset: offset
597611
}
598612
});
599613
}

src/vs/platform/userDataSync/test/common/settingsMerge.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,8 +1495,102 @@ suite('SettingsMerge - Add Setting', () => {
14951495

14961496
assert.equal(actual, sourceContent);
14971497
});
1498+
1499+
test('Insert after a comment with comma separator of previous setting and no next nodes ', () => {
1500+
1501+
const sourceContent = `
1502+
{
1503+
"a": 1
1504+
// this is comment for a
1505+
,
1506+
"b": 2
1507+
}`;
1508+
const targetContent = `
1509+
{
1510+
"a": 1
1511+
// this is comment for a
1512+
,
1513+
}`;
1514+
1515+
const expected = `
1516+
{
1517+
"a": 1
1518+
// this is comment for a
1519+
,
1520+
"b": 2
1521+
}`;
1522+
1523+
const actual = addSetting('b', sourceContent, targetContent, formattingOptions);
1524+
1525+
assert.equal(actual, expected);
1526+
});
1527+
1528+
test('Insert after a comment with comma separator of previous setting and there is a setting after ', () => {
1529+
1530+
const sourceContent = `
1531+
{
1532+
"a": 1
1533+
// this is comment for a
1534+
,
1535+
"b": 2,
1536+
"c": 3
1537+
}`;
1538+
const targetContent = `
1539+
{
1540+
"a": 1
1541+
// this is comment for a
1542+
,
1543+
"c": 3
1544+
}`;
1545+
1546+
const expected = `
1547+
{
1548+
"a": 1
1549+
// this is comment for a
1550+
,
1551+
"b": 2,
1552+
"c": 3
1553+
}`;
1554+
1555+
const actual = addSetting('b', sourceContent, targetContent, formattingOptions);
1556+
1557+
assert.equal(actual, expected);
1558+
});
1559+
1560+
test('Insert after a comment with comma separator of previous setting and there is a comment after ', () => {
1561+
1562+
const sourceContent = `
1563+
{
1564+
"a": 1
1565+
// this is comment for a
1566+
,
1567+
"b": 2
1568+
// this is a comment
1569+
}`;
1570+
const targetContent = `
1571+
{
1572+
"a": 1
1573+
// this is comment for a
1574+
,
1575+
// this is a comment
1576+
}`;
1577+
1578+
const expected = `
1579+
{
1580+
"a": 1
1581+
// this is comment for a
1582+
,
1583+
"b": 2
1584+
// this is a comment
1585+
}`;
1586+
1587+
const actual = addSetting('b', sourceContent, targetContent, formattingOptions);
1588+
1589+
assert.equal(actual, expected);
1590+
});
14981591
});
14991592

1593+
15001594
function stringify(value: any): string {
15011595
return JSON.stringify(value, null, '\t');
15021596
}

0 commit comments

Comments
 (0)