gnodet-bot commented on code in PR #26750:
URL: https://github.com/apache/camel/pull/26750#discussion_r4075779697
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java:
##########
@@ -364,6 +384,274 @@ public static JsonObject validate(ToolContext ctx, Path
dir, String file, String
/** How long a write waits for the running integration's reload record
before answering without it. */
static final long RELOAD_WAIT_MILLIS = 8000;
+ /**
+ * Replaces one snippet of a file and writes the result through {@link
#writeFile}, so a change to an existing file
+ * does not rewrite every line of it: a model that re-emits a whole file
corrupts the lines it did not mean to touch
+ * (CAMEL-24909). The snippet must occur exactly once; the answer says
what was replaced.
+ */
+ /** How long a file may be to be handed back when an edit misses, in
lines. */
+ private static final int MAX_EDIT_ECHO_LINES = 400;
+
+ public static JsonObject editFile(ToolContext ctx, Path dir, String file,
String find, String replace) {
+ JsonObject edit = editedContent(dir, file, find, replace);
+ String content = edit.getString("content");
+ if (content == null) {
+ return edit; // not-found or ambiguous: the answer says what to do
instead
+ }
+ JsonObject result = writeFile(ctx, dir, file, content, true);
+ if (!"invalid".equals(result.getString("status"))) {
+ result.put("status", "edited");
+ result.put("editedAtLine", edit.getInteger("editedAtLine"));
+ result.put("replacedLines", edit.getInteger("replacedLines"));
+ } else {
+ result.put("message", "The file was not changed: the result has
validation errors. Fix them and call"
+ + " camel_edit_file again.");
+ }
+ return result;
+ }
+
+ /**
+ * The content of the file with the snippet replaced, in {@code content},
with the line it changed and how many
+ * lines it replaced; or the answer of a miss (not-found, with the nearest
lines) or of an ambiguous snippet. The
+ * TUI writes that content itself, so an edit is confirmed and replayed in
the editor like a write.
+ */
+ public static JsonObject editedContent(Path dir, String file, String find,
String replace) {
+ Path path = resolveFile(dir, file);
+ if (!Files.isRegularFile(path)) {
+ throw new ToolExecutionException(file + " does not exist: write
the whole file with camel_write_file");
+ }
+ String content;
+ try {
+ content = Files.readString(path, StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ throw new ToolExecutionException("Failed to read " + path + ": " +
e.getMessage());
+ }
+ if (find == null || find.isEmpty()) {
+ throw new ToolExecutionException("find is required: the text to
replace, as it stands in the file");
+ }
+ String wanted = find;
+ String put = replace;
+ boolean trimmedMatch = false;
+ int first = content.indexOf(wanted);
+ int length = wanted.length();
+ if (first < 0) {
+ // the same lines with different indentation or trailing spaces: a
model composes the snippet from the
+ // shape it has in mind rather than from the file (CAMEL-24909),
so match on the trimmed lines when that
+ // names exactly one place
+ int[] window = uniqueTrimmedWindow(content, wanted);
+ if (window != null) {
+ first = window[0];
+ length = window[1] - window[0];
+ trimmedMatch = true;
+ }
+ }
+ if (first < 0 && hasLiteralEscapes(wanted)) {
+ // the snippet was built as a JSON string and its escapes were
left in it, so the text holds a literal
+ // \n where the file has a newline: read it the way it was meant
(CAMEL-24909)
+ String unescaped = unescapeLiterals(wanted);
+ int retry = content.indexOf(unescaped);
+ int retryLength = unescaped.length();
+ if (retry < 0) {
+ int[] window = uniqueTrimmedWindow(content, unescaped);
+ if (window != null) {
+ retry = window[0];
+ retryLength = window[1] - window[0];
+ trimmedMatch = true;
+ }
+ }
+ if (retry >= 0) {
+ first = retry;
+ length = retryLength;
+ wanted = unescaped;
+ put = unescapeLiterals(put);
+ }
+ }
+ JsonObject result = new JsonObject();
+ result.put("file", file);
+ if (first < 0) {
+ result.put("status", "not-found");
+ String nearest = nearestBlock(content, wanted);
+ String message = "The text to find is not in the file as given;
copy the lines from the file"
+ + (nearest != null ? ", which has there:\n" +
nearest : "");
+ if (nearest != null) {
+ result.put("nearest", nearest);
+ }
+ // a model that misses twice is writing the snippet from memory,
so hand it the file it is editing
+ // instead of sending it back to camel_get_files (CAMEL-24909)
+ if (content.lines().count() <= MAX_EDIT_ECHO_LINES) {
+ result.put("fileContent", content);
+ message += nearest != null
+ ? ". The whole file is in fileContent: copy the text
to find from there"
+ : ". The file as it stands is in fileContent: copy the
text to find from there";
+ } else if (nearest == null) {
+ message += " (camel_get_files reads it)";
+ }
+ result.put("message", message);
+ return result;
+ }
+ if (content.indexOf(wanted, first + wanted.length()) >= 0) {
+ result.put("status", "ambiguous");
+ result.put("occurrences", count(content, wanted));
+ result.put("message", "The text to find occurs more than once:
include the lines around it so it names one"
+ + " place, or write the whole file with
camel_write_file");
+ return result;
+ }
+ if (trimmedMatch) {
+ // the snippet was written at another indentation than the file
has: put the replacement in at the
+ // file's indentation, or the result is valid text at the wrong
depth (CAMEL-24909)
+ put = reindent(put, indentOf(wanted),
indentOf(content.substring(first)));
+ }
+ int line = (int) content.substring(0, first).lines().count()
+ + (first > 0 && content.charAt(first - 1) == '\n' ? 1 : 0);
+ result.put("content", content.substring(0, first) + put +
content.substring(first + length));
+ result.put("editedAtLine", Math.max(1, line));
+ // the lines actually replaced: with the trimmed match that is the
window in the file, which can be shorter
+ // than find when it ends in blank lines (CAMEL-24909)
+ result.put("replacedLines", (int) content.substring(first, first +
length).lines().count());
+ return result;
+ }
+
+ /** The leading whitespace of the first line of the text that has
something on it. */
+ private static String indentOf(String text) {
+ for (String line : text.split("\n", -1)) {
+ if (!line.isBlank()) {
+ int i = 0;
+ while (i < line.length() &&
Character.isWhitespace(line.charAt(i))) {
+ i++;
+ }
+ return line.substring(0, i);
+ }
+ }
+ return "";
+ }
+
+ /** Moves the text from the indentation it was written at to the one the
file has at that place. */
+ private static String reindent(String text, String from, String to) {
+ int delta = to.length() - from.length();
+ if (delta == 0 || text.isEmpty()) {
+ return text;
+ }
+ StringBuilder sb = new StringBuilder(text.length() + Math.abs(delta) *
8);
+ String[] lines = text.split("\n", -1);
+ for (int i = 0; i < lines.length; i++) {
+ String line = lines[i];
+ if (!line.isBlank()) {
+ if (delta > 0) {
+ line = " ".repeat(delta) + line;
+ } else {
+ int strip = 0;
+ while (strip < -delta && strip < line.length() &&
line.charAt(strip) == ' ') {
+ strip++;
+ }
+ line = line.substring(strip);
+ }
+ }
+ sb.append(line);
+ if (i < lines.length - 1) {
+ sb.append('\n');
+ }
+ }
+ return sb.toString();
+ }
+
+ /** Whether the text carries JSON escapes that were never turned back into
the characters they stand for. */
+ private static boolean hasLiteralEscapes(String text) {
+ return text != null && (text.contains("\\n") ||
text.contains("\\r\\n") || text.contains("\\t"));
+ }
+
+ /** Reads {@code \n}, {@code \r\n} and {@code \t} as the characters they
stand for. */
+ private static String unescapeLiterals(String text) {
+ return text == null ? null : text.replace("\\r\\n",
"\n").replace("\\n", "\n").replace("\\t", "\t");
+ }
+
+ /**
+ * The one place where the file's lines match the wanted lines once their
leading and trailing whitespace is
+ * removed, as start and end offset in the content, or null when there is
no such place or more than one.
+ */
+ private static int[] uniqueTrimmedWindow(String content, String find) {
+ List<String> wanted = find.lines().map(String::strip).toList();
+ while (!wanted.isEmpty() && wanted.get(wanted.size() - 1).isEmpty()) {
+ wanted = wanted.subList(0, wanted.size() - 1);
+ }
+ if (wanted.isEmpty()) {
+ return null;
+ }
+ String[] lines = content.split("\n", -1);
+ int[] offsets = lineOffsets(content, lines);
+ int[] found = null;
+ for (int i = 0; i + wanted.size() <= lines.length; i++) {
+ boolean match = true;
+ for (int j = 0; j < wanted.size(); j++) {
+ if (!lines[i + j].strip().equals(wanted.get(j))) {
+ match = false;
+ break;
+ }
+ }
+ if (match) {
+ if (found != null) {
+ return null; // more than one place: the caller must name
one
+ }
+ int end = offsets[i + wanted.size() - 1] + lines[i +
wanted.size() - 1].length();
+ found = new int[] { offsets[i], Math.min(end + 1,
content.length()) };
Review Comment:
⚠️ **Bug — trimmed-match window over-extends by one byte, losing the newline
separator when `replace` has no trailing newline.**
`end + 1` positions the window past the `\n` that terminates the last
matched line:
```
content[first : first+length] = " log: two\n" // includes \n
content[first+length :] = "route_c\n" // immediately next line
```
The substitution is `content.sub(0, first) + put + content.sub(first +
length)`. When `put` (the replacement) does not end with `\n` — which is the
common case; models write snippet text, not file terminators —
`content.sub(first + length)` starts right after the consumed `\n`, so the next
line runs directly onto the last line of `put`:
```
"route_a\n log: one\n\nroute_b\nlog: THREEroute_c\n done\n"
^^^^^^^^^^^^ glued
```
The current tests only exercise trimmed matches at the very end of the file
(`message: "two"` is the last content line in `ROUTE`), so `content.sub(first +
length)` is either empty or a single trailing `\n`, masking the issue.
The fix: exclude the `\n` from the window so the file's own newline is
preserved:
```suggestion
found = new int[] { offsets[i], Math.min(end,
content.length()) };
```
With this change, `content.sub(first + length)` starts at the `\n` itself,
which appears in the output as the separator between the replacement and the
following content. `replacedLines` is unaffected: `"B\nC".lines().count()` and
`"B\nC\n".lines().count()` both return 2.
A test to lock it in:
```java
@Test
void trimmedMatchInTheMiddleOfTheFilePreservesTheNewlineSeparator(@TempDir
Path dir) throws IOException {
// ROUTE has two routes; this replaces a line in the FIRST route,
// so the second route must still begin on its own line afterwards.
Files.writeString(dir.resolve("demo.camel.yaml"), ROUTE);
// model writes the snippet flat — trimmed match, non-terminal position
JsonObject result = edit(dir, "- log:\n message: \"one\"", "- log:\n
message: \"ONE\"");
assertThat(result.getString("status")).isEqualTo("edited");
String after = Files.readString(dir.resolve("demo.camel.yaml"));
// the second route must start on its own line, not glued to the
replacement
assertThat(after).contains("message: \"ONE\"\n");
assertThat(after.lines().count()).isEqualTo(ROUTE.lines().count());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
