p42ai / js-assistant

120+ refactorings and code-assists for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=p42ai.refactor
MIT License
119 stars 7 forks source link

Refactoring Idea: To/From Ternary #39

Closed hediet closed 1 year ago

hediet commented 1 year ago
let originalRange: LineRange;
if (c.originalEndLineNumber === 0) {
    // Insertion
    originalRange = new LineRange(c.originalStartLineNumber + 1, c.originalStartLineNumber + 1);
} else {
    originalRange = new LineRange(c.originalStartLineNumber, c.originalEndLineNumber + 1);
}

=>

let originalRange: LineRange = c.originalEndLineNumber === 0
    // Insertion
    ? new LineRange(c.originalStartLineNumber + 1, c.originalStartLineNumber + 1)
    : new LineRange(c.originalStartLineNumber, c.originalEndLineNumber + 1);

Context:


export class SmartLinesDiffComputer implements ILinesDiffComputer {
    computeDiff(originalLines: string[], modifiedLines: string[], options: ILinesDiffComputerOptions): ILinesDiff {
        const diffComputer = new DiffComputer(originalLines, modifiedLines, {
            maxComputationTime: options.maxComputationTime,
            shouldIgnoreTrimWhitespace: options.ignoreTrimWhitespace,
            shouldComputeCharChanges: true,
            shouldMakePrettyDiff: true,
            shouldPostProcessCharChanges: true,
        });
        const result = diffComputer.computeDiff();
        return {
            quitEarly: result.quitEarly,
            changes: result.changes.map(c => {
                let originalRange: LineRange;
                if (c.originalEndLineNumber === 0) {
                    // Insertion
                    originalRange = new LineRange(c.originalStartLineNumber + 1, c.originalStartLineNumber + 1);
                } else {
                    originalRange = new LineRange(c.originalStartLineNumber, c.originalEndLineNumber + 1);
                }

                let modifiedRange: LineRange;
                if (c.modifiedEndLineNumber === 0) {
                    // Deletion
                    modifiedRange = new LineRange(c.modifiedStartLineNumber + 1, c.modifiedStartLineNumber + 1);
                } else {
                    modifiedRange = new LineRange(c.modifiedStartLineNumber, c.modifiedEndLineNumber + 1);
                }

                return {
                    originalRange,
                    modifiedRange,
                    innerChanges: c.charChanges?.map(c => new RangeMapping(
                        new Range(c.originalStartLineNumber, c.originalStartColumn, c.originalEndLineNumber, c.originalEndColumn),
                        new Range(c.modifiedStartLineNumber, c.modifiedStartColumn, c.modifiedEndLineNumber, c.modifiedEndColumn),
                    ))
                };
            })
        };
    }
}
lgrammel commented 1 year ago

Thanks for the suggestion and the great bug report, made it really easy to reproduce it!

P42 currently already supports this as a 2-step refactoring:

  1. Convert to conditional expression
  2. Merge initialization and declaration

https://user-images.githubusercontent.com/205036/178684186-6eea111d-f0ea-48d3-9860-0625fef1c0a2.mov

There seems to be a small bug where the // Insertion comment gets lost.

Is the idea about extending the convert to conditional expression refactoring to automatically perform the merge of initialization/declaration?

hediet commented 1 year ago

I think I somehow missed that action.