Skip to content

Retain Manifest Formatting on pack Command #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ChekTek wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Retain Manifest Formatting on pack Command #65

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7719904
preserve manifest line endings
ChekTek Jan 9, 2026
59c1820
preserve indentation style for manifest
ChekTek Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/commands/pack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,25 @@ async function version(path: string, version: string | null): Promise<VersionRev
original = await readFile(manifestPath, { encoding: "utf-8" });
const manifest = JSON.parse(original) as Partial<Manifest>;

// Detect the original line ending style (CRLF or LF)
const lineEnding = original.includes("\r\n") ? "\r\n" : "\n";

// Detect the original indentation style (tabs or spaces)
const indentMatch = original.match(/^[\t ]+/m);
const indent = indentMatch?.[0].startsWith("\t") ? "\t" : indentMatch?.[0] ?? "\t";
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation detection logic is complex and unclear. If the match finds spaces, it uses all matched spaces as the indent, but if it finds tabs, it only uses a single tab. This inconsistency could lead to incorrect indentation if the original file uses multiple tabs. Consider explicitly handling the tab case to use the full matched string, or document why single tab is preferred.

Suggested change
const indent = indentMatch?.[0].startsWith("\t") ? "\t" : indentMatch?.[0] ?? "\t";
const indent = indentMatch?.[0] ?? "\t";

Copilot uses AI. Check for mistakes.

// Ensure the version in the manifest has the correct number of segments, [{major}.{minor}.{patch}.{build}]
version ??= manifest.Version?.toString() || "";
manifest.Version = `${version}${".0".repeat(Math.max(0, 4 - version.split(".").length))}`;
write(JSON.stringify(manifest, undefined, "\t"));

let stringified = JSON.stringify(manifest, undefined, indent);

// Preserve original line endings
if (lineEnding === "\r\n") {
stringified = stringified.replaceAll("\n", "\r\n");
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using replaceAll to convert LF to CRLF will cause issues if the stringified content already contains any CRLF sequences (which could happen if manifest values contain embedded newlines). This would result in CRCRLF sequences. Consider using replace with a regex that specifically targets LF not preceded by CR: stringified.replace(/(?<!\r)\n/g, '\r\n')

Suggested change
stringified = stringified.replaceAll("\n", "\r\n");
stringified = stringified.replace(/(?<!\r)\n/g, "\r\n");

Copilot uses AI. Check for mistakes.
}

write(stringified);
}

return {
Expand Down