Added capability to applypatch -> replaceText to replace the header along with the content #2
Conversation
coddingtonbear
left a comment
coddingtonbear
left a comment
There was a problem hiding this comment.
This is a great start, but I'm a little unsure about what you're trying to do in cli.ts, so I have some comments there.
| .option( | ||
| "--replace-heading", | ||
| "Replace the heading as well as its content." | ||
| ) |
There was a problem hiding this comment.
It seems a little bit surprising to me that this would be a global option for the patch applier when that particular option is only valid for certain kinds of patch instructions; what was your intention here?
| if (operation === "replace" && targetType === "heading") { | ||
| instruction = { | ||
| operation, | ||
| targetType, | ||
| content, | ||
| target: target.split(options.delimiter), | ||
| replaceHeading: options.replaceHeading, | ||
| }; | ||
| } else { | ||
| // If the user tries to use the flag incorrectly, exit with an error. | ||
| if (options.replaceHeading) { | ||
| console.error( | ||
| "The --replace-heading flag is only valid for 'replace' operations on 'heading' targets." | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| // Create other instruction types as before | ||
| instruction = { | ||
| operation, | ||
| targetType, | ||
| content, | ||
| target: | ||
| targetType !== "heading" ? target : target.split(options.delimiter), | ||
| } as PatchInstruction; | ||
| } |
There was a problem hiding this comment.
I think the answer to the above question will probably help me understand the answer to this one, but in case it doesn't: I'm finding myself wondering why we can't just read the replaceHeading value out of the incoming PatchInstruction.
|
Closing due to this PR appearing to be abandoned. Please reach out if you're open to continuing work on this, and I'd be glad to re-open. |
2 participants
Had a bit more to change here. Added some typeguard to check for "heading" and "replace" in the instructions.
other than that, in the replaceText function, it checks if replaceHeading is true and change the start of the first slice.
const replaceText = ( document: string, instruction: PatchInstruction, target: DocumentMapMarkerContentPair ): string => { let start = target.content.start; if ( isReplaceHeadingPatchInstruction(instruction) && instruction.replaceHeading ) { start = target.marker.start; } return [ document.slice(0, start), instruction.content, document.slice(target.content.end), ].join(""); };Note that there needs to be a new line between the header and the content to this properly all in the same content.
A future change could be to separate the header and the content, but felt that this would have the most minimal changes.