Extract reconcile #85

2025-07-12T12:59:48+01:00

schmelczer commented

2025-07-12 12:59:48 +01:00

(Migrated from github.com)

Split off https://github.com/schmelczer/reconcile into its own repo and depend on it as a package

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-07-12 13:04:00 +01:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR extracts the in-repo reconcile implementation into the external reconcile-text crate and updates merge logic across server and client to use this new dependency. It also adds a utility to detect mergeable file types and adjusts module exports.

Remove the internal reconcile and sync_lib crates in favor of reconcile-text
Introduce is_file_type_mergable, update server and client to use reconcile for 3-way merges on text
Export relevant reconcile-text types in client API and clean up tests and imports

Reviewed Changes

Copilot reviewed 93 out of 136 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
sync-server/src/utils/is_filetype_mergable.rs	Add utility to detect `.md`/`.txt` files
sync-server/src/server/update_document.rs	Replace inline merge with `reconcile-text::reconcile`
sync-server/src/server.rs	Reformat single-line handlers
sync-server/src/utils.rs	Register new `is_filetype_mergable` module
frontend/sync-client/src/file-operations/filesystem-operations.ts	Swap out in-house merge for `reconcile-text` types and APIs
frontend/sync-client/src/utils/is-file-type-mergable.ts	Mirror server utility in client
frontend/sync-client/src/index.ts	Re-export `reconcile-text` types

Files not reviewed (1)

frontend/package-lock.json: Language not supported

Comments suppressed due to low confidence (2)

sync-server/src/utils/is_filetype_mergable.rs:1

[nitpick] The function name is_file_type_mergable uses an underscore between "file" and "type", but the module/file is named is_filetype_mergable. Consider unifying the naming (either remove the underscore or rename the module) for consistency.

pub fn is_file_type_mergable(path_or_file_name: &str) -> bool {

sync-server/src/server.rs:187

client_error is used here but not imported; add use crate::errors::client_error; (or include it in the existing import) to avoid a compilation error.

    client_error(anyhow!("Method not allowed"))

## Pull Request Overview This PR extracts the in-repo reconcile implementation into the external `reconcile-text` crate and updates merge logic across server and client to use this new dependency. It also adds a utility to detect mergeable file types and adjusts module exports. - Remove the internal `reconcile` and `sync_lib` crates in favor of `reconcile-text` - Introduce `is_file_type_mergable`, update server and client to use `reconcile` for 3-way merges on text - Export relevant `reconcile-text` types in client API and clean up tests and imports ### Reviewed Changes Copilot reviewed 93 out of 136 changed files in this pull request and generated 1 comment. <details> <summary>Show a summary per file</summary> | File | Description | | ------------------------------------------------------------ | ------------------------------------------------------------------ | | sync-server/src/utils/is_filetype_mergable.rs | Add utility to detect `.md`/`.txt` files | | sync-server/src/server/update_document.rs | Replace inline merge with `reconcile-text::reconcile` | | sync-server/src/server.rs | Reformat single-line handlers | | sync-server/src/utils.rs | Register new `is_filetype_mergable` module | | frontend/sync-client/src/file-operations/filesystem-operations.ts | Swap out in-house merge for `reconcile-text` types and APIs | | frontend/sync-client/src/utils/is-file-type-mergable.ts | Mirror server utility in client | | frontend/sync-client/src/index.ts | Re-export `reconcile-text` types | </details> <details> <summary>Files not reviewed (1)</summary> * **frontend/package-lock.json**: Language not supported </details> <details> <summary>Comments suppressed due to low confidence (2)</summary> **sync-server/src/utils/is_filetype_mergable.rs:1** * [nitpick] The function name `is_file_type_mergable` uses an underscore between "file" and "type", but the module/file is named `is_filetype_mergable`. Consider unifying the naming (either remove the underscore or rename the module) for consistency. ``` pub fn is_file_type_mergable(path_or_file_name: &str) -> bool { ``` **sync-server/src/server.rs:187** * `client_error` is used here but not imported; add `use crate::errors::client_error;` (or include it in the existing import) to avoid a compilation error. ``` client_error(anyhow!("Method not allowed")) ``` </details>

sync-server/src/server/update_document.rs

					
				@ -0,0 +117,4 @@

				        return Ok(Json(DocumentUpdateResponse::FastForwardUpdate(

				            latest_version.into(),

				        )));

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-07-12 13:04:00 +01:00

The code only reconciles when is_binary(...) is true, but then treats the content as UTF-8 text. It should use !is_binary(...) to ensure reconciliation only runs on text data.

        && !is_binary(&parent_document.content)
        && !is_binary(&latest_version.content)
        && !is_binary(&content)

The code only reconciles when `is_binary(...)` is true, but then treats the content as UTF-8 text. It should use `!is_binary(...)` to ensure reconciliation only runs on text data. ```suggestion && !is_binary(&parent_document.content) && !is_binary(&latest_version.content) && !is_binary(&content) ```

Sign in to join this conversation.

No reviewers