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

git merge? #245

Open
arcxne opened this issue Aug 28, 2024 · 12 comments
Open

git merge? #245

arcxne opened this issue Aug 28, 2024 · 12 comments

Comments

@arcxne
Copy link

arcxne commented Aug 28, 2024

Hey there, I'm loving gitu so far! Just wanted to ask if git merge could be implemented. Maybe keybind 'm' could work?

Thanks and best wishes!

@altsem
Copy link
Owner

altsem commented Aug 28, 2024

It should be! And relatively easy.

Could be a good first task for anyone interested in implementing :) A lot of code would be similar to the rebase module I imagine.

@arcxne
Copy link
Author

arcxne commented Aug 29, 2024

Actually, could I try implementing it myself this weekend? This project is really cool and I want to try contributing to it, and I can learn Rust at the same time :)

@altsem
Copy link
Owner

altsem commented Aug 31, 2024

Feel free to give it a go and open a PR/draft. I'd try help out if you get stuck!

@arcxne
Copy link
Author

arcxne commented Aug 31, 2024

Thanks for the reassurance! I'm trying to workout the file structure so far, but I don't really understand how things are organised. Is there anywhere I can read on this? Or maybe you could help briefly explain the structure?

I think I got way too ahead of myself when I decided to take on this challenge, but that's where the learning happens!

@arcxne
Copy link
Author

arcxne commented Aug 31, 2024

Seems like the bulk of it is in /src/ops! I'll create a merge.rs file there and get to work on it 🖥️

@altsem
Copy link
Owner

altsem commented Aug 31, 2024

That seems about right! You could add a keybind in the default config and see where it breaks from there.
There's a bunch of tests being ran if you do 'cargo test' too.

@arcxne
Copy link
Author

arcxne commented Aug 31, 2024

Current plan:

  • if uncommitted/unstashed changes, warn user and abort
  • assume current branch to be target branch. make this known to user (next bullet)
  • ask user for branch to be merged. The prompt will be something like "Branch to merge into : "
  • if any conflicts, let user know, User can choose to
    • resolve conflicts (opens git mergetool?)
    • abort and go back to pre-merge state (hence why committing/stashing is req) (git merge --abort)
    • (i have no idea what git merge --continue does but i think it's useless?)
  • merge changes and open commit msg panel (latter is alr in gitu)
  • commit!

Should an option to run merge without committing (git merge --no-commit) be added? Or maybe instead of forcing a commit msg panel, user can simply be notified that merge is ready to be committed and manual commit wld be req.

EDIT: Was trying out the rebase cmd and it's seems much better to follow for merge too. So I might not do the above, will see

@arcxne
Copy link
Author

arcxne commented Aug 31, 2024

That seems about right! You could add a keybind in the default config and see where it breaks from there. There's a bunch of tests being ran if you do 'cargo test' too.

Sure! I'll try that out. On the topic of tests, may I know what tests are exactly and how they are supposed to be used? I see test folders in diff repos but don't really know how they help in 'testing'.

@altsem
Copy link
Owner

altsem commented Aug 31, 2024

I'm a bit busy this weekend but, I'd try sit down and see how Magit behaves, and try replicate as much as it makes sense :) The tests try simulate real usage to try keep bugs away. I can cook some tests for you at the end, don't have to worry about it if you don't want.

@arcxne
Copy link
Author

arcxne commented Sep 1, 2024

Ah, that explanation about tests makes sense.

Don't worry about me, I'll try my best!

@arcxne
Copy link
Author

arcxne commented Sep 5, 2024

Hey there, been working on and off on this. I realise that I should have asked this earlier, but how am I to test out what I have done so far? Even better if I can do so without overriding the existing gitu version. Will make test do the trick? I'm getting 422 errors with it currently 😅

Edit: cargo run --release seems to do the trick

@altsem
Copy link
Owner

altsem commented Sep 6, 2024

Hey @arcxne.

It's a bit hard without more specifics, but I suspect some of the test snapshots may need updating if you add a new entry in the menu.
Have a look at "cargo insta": https://insta.rs/docs/cli/

I would copy the existing test, and start from there: https://github.com/altsem/gitu/blob/master/src/tests/rebase.rs

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

No branches or pull requests

2 participants