Skip to content

Conversation

useche
Copy link

@useche useche commented Jun 29, 2025

Mercurial follows a very similar patter to Git: (1) It has an index pointing to the latest changes in the repo (dirstate) and (2) we are able to cat the file content of files in the repository.

This patch does three two things:

  1. Format diff.lua with lua_ls. If this is not ok, I can add a revert of it.
  2. Parameterize Git handling so that it accept a generic index, command to find the repo directory, and command to get files in the repo. Methods are renamed s/git_/dvcs_/ to make it clear that they are more generic now.
  3. Add mercurial support by populating the parameters created above with the correct values.

useche added 4 commits June 29, 2025 12:13
the final idea is to add support for `mercurial`, that has a similar
structure to git's. Parameterizing these functions makes the addition of
mercurial trivial. The patch renames those functions as `s/git_/dvcs_/`
to reflect their more generic nature.
so that next time we try to use `dvcs_cache`, we can start from scratch.
This is important when other `gen_sources` are using `dvcs_cache` as
well.
@krovuxdev
Copy link

@useche This can be modified here.

@useche
Copy link
Author

useche commented Jun 30, 2025

@useche This can be modified here.

I think you mean that I could implement my own source in my init.lua. This is true, but I will end up repeating a lot of code that's already in MiniDiff.

Instead, this patch parameterize the three things that are different between Git and Mercurial and then makes it trivial to implement the mercurial side. All we need to implement the mercurial diff is:

    local watch_index_opts = {
      command = 'hg',
      dvcs_dir_cmd_args = { 'root', '--template', '{reporoot}/.hg' },
      index_name = 'dirstate',
      dvcs_file_test_spawn_args_fn = function(local_path)
        local basename = vim.fn.fnamemodify(local_path, ':t')
        return { 'cat', basename }
      end
    }
    H.dvcs_start_watching_index(buf_id, path, watch_index_opts)

We can now reuse the code to diff with Git which is well tested.

The patch looks large because there are a lot of renaming to reflect the fact that some things are more generic now and also because I formatted the file with lua_ls (I'll remove it to make things easier to review).

@echasnovski
Copy link
Member

Thanks for the PR!

Adding built-in support for Mercurial, Jujutsu, and maybe Fossil is indeed planned. The problem here is that I don't use any of these, but I would like to be able to personally verify that the solution works. It requires some time investment from my side to understand how they work and how to test them. So at least a set of commands that are verified from the actual user of the VCS is highly appreciated!

As for the PR, I don't really agree with several refactoring choices, in particular naming (I'd prefer vcs instead of dvcs, more concise names of watch_index_opts fields, just opts instead of watch_index_opts, and probably mercurial source name instead of hg) and some modifications of not directly affected code (mostly some nit picks).

Formatting of the project is done with 'StyLua' (which is both documented and could be inferred from the fact that affected lines are mostly the ones that have --stylua: ignore near them). So yes, extra formatting with lua_ls is not welcome.


Also, before making a PR, please create an issue/discussion about adding particular functionality, so that we can discuss it first. This usually saves time of both parties.

What I'd suggest is the best approach going forward is to ask you (as evidently a Mercurial user) to help me with a testing setup (like, a set of commands to initiate a testing repo) and a set of commands for basic actions:

  1. How to test if inside a Mercurial repo.
  2. How to setup reacting to changes in index.
  3. How to get text of particular file inside index.
  4. How to create and apply patch for apply_hunks.

My understanding from the PR is the following:

  1. Running hg root --template {reporoot}/.hg returns an error if not in repo.
  2. If index changes, the file '/path/to/.hg/dirstate' changes (where '/path/to/.hg' is the output of the previous successful step).
  3. hg cat rel/path/to/file
  4. Nothing in the PR so far.

in particular, this patch fixes:
1. `s/watch_index_opts/opts/`
2. `s/dvcs/vcs/`
3. `s/hg/mercurial/`
4. Format with `stylua`
@useche
Copy link
Author

useche commented Jun 30, 2025

Thanks for the PR!

Thank you for taking a look.

Adding built-in support for Mercurial, Jujutsu, and maybe Fossil is indeed planned. The problem here is that I don't use any of these, but I would like to be able to personally verify that the solution works. It requires some time investment from my side to understand how they work and how to test them. So at least a set of commands that are verified from the actual user of the VCS is highly appreciated!

I'm happy to help.

As for the PR, I don't really agree with several refactoring choices, in particular naming (I'd prefer vcs instead of dvcs, more concise names of watch_index_opts fields, just opts instead of watch_index_opts, and probably mercurial source name instead of hg) and some modifications of not directly affected code (mostly some nit picks).

I made the changes you mentioned here. Let me know if I need to change anything else.

Formatting of the project is done with 'StyLua' (which is both documented and could be inferred from the fact that affected lines are mostly the ones that have --stylua: ignore near them). So yes, extra formatting with lua_ls is not welcome.

Code formatted with stylua.

Also, before making a PR, please create an issue/discussion about adding particular functionality, so that we can discuss it first. This usually saves time of both parties.

Sounds good for next time.

What I'd suggest is the best approach going forward is to ask you (as evidently a Mercurial user) to help me with a testing setup (like, a set of commands to initiate a testing repo) and a set of commands for basic actions:

  1. How to test if inside a Mercurial repo.
  2. How to setup reacting to changes in index.
  3. How to get text of particular file inside index.
  4. How to create and apply patch for apply_hunks.

My understanding from the PR is the following:

  1. Running hg root --template {reporoot}/.hg returns an error if not in repo.
  2. If index changes, the file '/path/to/.hg/dirstate' changes (where '/path/to/.hg' is the output of the previous successful step).
  3. hg cat rel/path/to/file

To create a fresh repository, similar to git, you would do hg init. Once that's done, you are able to commit changes. Again, similar to git, hg commit -m "my message" would commit a message.

As for what's necessary to monitor the repository, the commands you wrote are the ones I identified to monitor the mercurial repository. Those are the ones I'm using in the patch.

  1. Nothing in the PR so far.

What do you mean by thise?

@useche
Copy link
Author

useche commented Jun 30, 2025

By the way, looking at Jujutsu structure, it seems like we can use the structure that I put in place with the following parameters:

  1. jj root should print the .jj directory of the repository.
  2. The file .jj/repo/op_heads/default changes every time there is a new commit, so we can monitor that one.
  3. jj cat will print a file text as it is in the repo.

This was a 5 minutes search, so take it with a grain of salt. But it looks possible.

@echasnovski
Copy link
Member

7. Nothing in the PR so far.

What do you mean by thise?

The source (if possible and makes sense) should also supply apply_hunks method. For various VCS this is something like "update file index based on these hunks". This is possible with git source by constructing and applying patch, so I'd assume something similar should be possible with Mercurial and Jujutsu.

@useche
Copy link
Author

useche commented Jun 30, 2025

  1. Nothing in the PR so far.

What do you mean by thise?

The source (if possible and makes sense) should also supply apply_hunks method. For various VCS this is something like "update file index based on these hunks". This is possible with git source by constructing and applying patch, so I'd assume something similar should be possible with Mercurial and Jujutsu.

As far as I know, staging pieces of the patch in an index before creating a commit is something pretty unique to Git.

In mercurial, you are able to commit pieces of the outstanding changes with hg commit -p. You can also amend a commit with hg amend -p. But the only way to include something in the repository is with a commit. I don't think this is what we want in mini.diff. Right? It's probably better left unimplemented as it doesn't really make sense the way it does in Git.

For jujutsu, we can discuss it in more detail when we implement that. But, IIRC, jujutsu takes any opportunity it has (every time jj is called) to stage any changes in the working directory. If you want to create a commit with pieces of that staged data, you have to select which ones at commit time.

@echasnovski
Copy link
Member

As far as I know, staging pieces of the patch in an index before creating a commit is something pretty unique to Git.

That's a shame. I'll need to do more research then to see the best way to utilize apply_hunks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants