Skip to content
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

tools: add diffedit3 as a dotslash tool #4295

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thoughtpolice
Copy link
Member

@thoughtpolice thoughtpolice commented Aug 16, 2024

This adds diffedit3 as a DotSlash file in the repository so that you can get a version of diffedit3 automatically on every platform, including Windows.

This is a piece of my buck2 branch, and it might be more widely useful, so I'm submitting it here if anyone wants to give it a shot. (Note that the Buck2 build currently does rely on this functionality, but I think that this should be considered independently of that.)

With this you can run jj config edit --repo and add:

[merge-tools.diffedit3]
program = "tools/bin/diffedit3"

then run jj diffedit in order to use the included binary without having to install it globally.

Nit: it would be nice if we could include something like a $root variable in the merge-tools.tool.program field; the above example only works when you run it from jj root. But that could be fixed separately.

@thoughtpolice thoughtpolice self-assigned this Aug 16, 2024
This ensures that they can't bitrot and at least launch on all platforms.

Signed-off-by: Austin Seipp <[email protected]>
@thoughtpolice thoughtpolice force-pushed the aseipp/push-vzypxsolopto branch from 8c9964c to 7f91da2 Compare August 16, 2024 21:14
@ilyagr
Copy link
Contributor

ilyagr commented Aug 20, 2024

I said most of this on Discord, but repeating myself, I think using DotSlash just for diffedit3 might not be the best. However, this could be quite nice if we require more devtools to develop jj.

Currently, the only such tool of note is poetry for building the docs, and the main limitation is that the user has to install a Python distribution, then pipx, and then poetry. I'm not sure whether DotSlash can handle installing a Python interpreter and poetry together. (I looked at the docs briefly, it doesn't seem like it'd be trivial, though it's probably doable if somebody provides an appropriate package of the two together for each platform.) However, I've already been thinking about switching to https://rye.astral.sh/, which is a single binary that can setup a Python interpreter by itself, and should be easy to install with DotSlash.

Another tool we could consider is something to format our Markdown files, probably https://dprint.dev/. See also #3757 (comment) .

I'm not 100% sure whether we should recommend using DotSlash or just installing such programs (these are all single-file binaries). If we do use DotSlash, we might also want to use a Cargo xtask or something to run the tools and to avoid the user needing to think about the paths to them.

And once/if we are using DotSlash for something else, adding diffedit3 to it might be nice too!

@ilyagr
Copy link
Contributor

ilyagr commented Aug 20, 2024

One seeming limitation of DotSlash is that they don't seem to support an easy way of keeping the JSON config up to date. They provide a GitHub action that a project on GitHub can use to provide a config to their user (so, I could make diffedit3 do that), but I'm not sure if there's a way to do it for projects that we do not control. We could make a tool for that ourselves, but then we'd have to support it...

@thoughtpolice
Copy link
Member Author

One seeming limitation of DotSlash is that they don't seem to support an easy way of keeping the JSON config up to date. They provide a GitHub action that a project on GitHub can use to provide a config to their user (so, I could make diffedit3 do that), but I'm not sure if there's a way to do it for projects that we do not control. We could make a tool for that ourselves, but then we'd have to support it...

TBH, I've thought about just writing a Deno script that can do this for some projects. It might be worth it, I'm not sure. But I agree it's a problem.

@thoughtpolice
Copy link
Member Author

I'm not 100% sure whether we should recommend using DotSlash or just installing such programs (these are all single-file binaries). If we do use DotSlash, we might also want to use a Cargo xtask or something to run the tools and to avoid the user needing to think about the paths to them.

For some tools (like buck2 in the buck branch) I think it's really important to install DotSlash and use the exact versions, and it's also a lot easier to keep things up to date if you encourage people to do that.

However, I think it's important to note these tools are basically only useful for developers, and probably frequent developers at that. So "just install DotSlash" probably isn't too bad, considering it's also a Rust tool.

Also, I currently put the following inside my jj .envrc (via direnv):

export PATH=$(jj root)/tools/bin:$PATH

Which always makes the tools available in my $PATH, so I can immediately run buck2. Something like direnv for Windows would be nice, but it's a bit out of scope for now...

@ilyagr
Copy link
Contributor

ilyagr commented Sep 5, 2024

Something like direnv for Windows would be nice, but it's a bit out of scope for now...

@thoughtpolice , I haven't used it, but https://github.com/direnv/direnv/releases seems to have direnv for Windows. I'm not sure how well it works, but it claims to support PowerShell.

@thoughtpolice
Copy link
Member Author

Very interesting. Unfortunately, looking through the bug tracker it seems there are at least half a dozen major Windows issues that probably mean it won't work for us (e.g. bash requirement, mangling env var names) until those are fixed. But I'll keep my eye on it.

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

Successfully merging this pull request may close these issues.

3 participants