Skip to content

Add diff support for mercurial (hg) repos #9372

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.

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
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zegervdv
Copy link
Contributor

Adds diff and branch name support for mercurial (hg) repositories.

I'm using the rhg command for faster execution, but that is currently not installed by default.
The speed-up brings it down from ~750ms overhead on startup to ~50ms, so definitely noticeable.
But it breaks the portability for users or systems that only have the commonly used hg command installed.

Perhaps a config option might allow the user to decide to use the faster rhg or fall back to hg?

@kirawi
Copy link
Member

kirawi commented Jan 17, 2024

We don't want to support any VCS other than git in core. Instead, we want support for other systems to be implemented via plugins in the future.

@zegervdv
Copy link
Contributor Author

Oh, I see. In the meantime I'll keep using my fork then :)

@zegervdv zegervdv closed this Jan 19, 2024
@archseer
Copy link
Member

I think we should at least take a look, @kirawi's tone was too strong here. I like how small the implementation is but I wonder about performance

@archseer archseer reopened this Jan 19, 2024
@kirawi
Copy link
Member

kirawi commented Jan 19, 2024

Sorry about that, I misremembered this (thought you were totally against it): https://matrix.to/#/!zMuVRxoqjyxyjSEBXc:matrix.org/$mish8TCtq3pO9r2Rr0V9IH7c3PG89aDKroXpvW5DhsA?via=matrix.org&via=mozilla.org&via=envs.net

@pascalkuthe
Copy link
Member

I am sceptic about merging support for other VCS into core

  • the testcases in this PR fail CI (and will also fail for almost all developers locally) because they require mecurial binaries to be present.
  • I have a lot of plans regarding the evolution of diff support (diff against stash, diff against commits and other buffers) having a second VCS would require me to apply that same effort to that second VCS too which would increase the required effort a lot
  • there are some subtelties here that aren't properly considered (encodings and line endings). I know a lot about git to maintain and handle these edgecases there but the same is not true for mecurial
  • this is an API where its very easy to expose a plugin interface and this implemention shells out. That is just as easy to do in scheme as in rust
  • adding support for everything in core blots compiletime. With firefox moving to git (and the mecurial project itself slowing donw). I think mecurial (and generally any no-git VCS) is niche enough to relegate to plugins (especially because it would be quite easy to implement as a plugin)

@zegervdv
Copy link
Contributor Author

Thanks for all your comments.

I understand this would add additional maintenance burden. And the fact that it requires hg to be installed makes it not portable.
I have a variant that works with the official hg-core rust library, but since it is not published and (of course) stored in a mercurial repository it can only build if the repo is at a relative location, so that is also not an option. The hg-core library is also still very much in development and the API is not stable to build a tool on. I'm not sure where you read the project is slowing down, from what I can see there is still a lot of activity and interesting new ideas being worked on.

I don't mind reimplementing this as a plugin when that eventually becomes available. It would be a nice-to-have if those could run asynchronously to avoid seeing some of the latency when shelling out.

Until then I'll keep applying my patches and try to keep up with the stable releases.

FYI @archseer I did some benchmarks on my machine (i5-1135G7) and saw that running an hg command takes between 200ms and 400ms.
To make sure I'm in the correct repository, check if the file is tracked and get the file content for diffing, I need 3 commands, so that adds about 1 second to the startup of helix. This is definitely noticeable (and frankly annoying :)).
So I switched to the rhg binary, which is an experimental pure rust implementation and got times of about 15ms per run, this means an extra startup time of about 50ms.

So anyway, thanks for your time and I'm looking forward to the plugin system :)

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Jan 23, 2024
@pascalkuthe pascalkuthe added the A-plugin Area: Plugin system label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements A-plugin Area: Plugin system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants