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-commit-mode prevents magit from running certain [not "any"] other git operations in any repo #827

Closed
aspiers opened this issue Aug 29, 2013 · 9 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@aspiers
Copy link
Contributor

aspiers commented Aug 29, 2013

Once launched via git running emacsclient, git-commit-mode effectively locks the use of git until the commit is completed or aborted. This is my single biggest complaint about this new commit mechanism, and it does make me suspect that the modal-like nature of this design is fundamentally flawed.

Example use case which is broken by this design:

  1. initiate a commit from magit
  2. type a long commit message
  3. suddenly realise that you are about to commit to the wrong branch
  4. switch back to magit status buffer
  5. press b b to switch to the correct branch
  6. enter which branch to switch to
  7. boom! get an error: Git is already running

Workaround: switch branch from CLI instead :-/

Another example use case:

  1. start committing something in one repo
  2. someone interrupts you and asks you to work on another codebase
  3. you can't do anything in that other codebase until you complete or abort the original commit

Workaround: save the message somewhere then abort the commit, or complete it and then fix branch tips :-/

One could argue that the new workflow encourages developers to adopt a "cleaner" workflow, where open tasks are not left open for a long period of time. This is true, but IMHO it should be up to the user to decide their own workflow. The tool is there to serve the user, not the other way around, so it should not be so opinionated about how its user works.

Other breakage caused by this new design include #772, #785, and #805 - mostly resolved already (great work!) so hopefully this is teething problems rather than an indication that something is more fundamentally broken.

@aspiers
Copy link
Contributor Author

aspiers commented Aug 30, 2013

I previously suggested a solution which was to teach magit how to handle multiple git processes at once. However after more thought triggered by discussion on the google group, I think a better solution is "don't start git commit until the commit log editing has been completed". This avoids the problems which stem from the modal-like nature of the current workflow.

A non-modal approach also satisfies the "long-lived commit buffer" workflow.

@vanicat
Copy link
Member

vanicat commented Aug 30, 2013

Adam Spiers notifications@github.com writes:

I previously suggested a solution which was to teach magit how to
handle multiple git processes at once. However after more thought
triggered by discussion on the google group, I think a better solution
is "don't start git commit until the commit log editing has been
completed
".
This avoids the problems which stem from the modal-like nature of the
current
workflow
.

The problem is that if we don't use git to prepare the commit message,
we lose all normal git machinery that one can use, as commit.template
and githooks(5).

Generally speaking, we definitively should find some solution to run more
git command concurrently.

Rémi Vanicat

@tarsius
Copy link
Member

tarsius commented Aug 30, 2013

Generally speaking, we definitively should find some solution to run more git command concurrently.

Yes, only allowing one git processes is in may cases not necessary. Lets discuss that on #24, the oldest still open issue! I just recently changed that from feature-request to bug, in case you are wondering how a bug does not get fixed in such a long time.

modal-like nature of the current workflow

That wasn't intentional. And it should be "easy" enough to fix without first refactoring (see above/#24) another core component of Magit or dropping the client thing. Use start-file-process directly instead of magit-run-git-async and by extension magit-run*. That would bypass the latter's one-process assert but also remove some generic error handling. Whether that is a problem or not I am not sure yet.

So maybe some refactoring is needed in that area even before we make functional changes (#24) to accommodate this change. One such thing I have started a while back but never completed (or just not sufficiently tested?) is to factor out the duplicated (but strangely somewhat different) error handling in magit-run* and magit-process-sentinel. If some one wants to have a stab at it I could push the unfinished work somewhere.

@tarsius
Copy link
Member

tarsius commented Aug 30, 2013

I am factoring the whole "making sure we find emacsclient and have a fallback that works very similarly if we cannot use that" functionality out into a new library with-editor. That library also absorbs the finish and cancel commands from git-{commit,rebase}-mode functionality, adding hooks to account for differences. That allows removing duplicated in commit/rebase. It also features a minor and a major mode, which commit/rebase use to further reduce duplication - or anything else (outside Magit even) that needs a client.

Some of these things still have to be fleshed out but it basically works already. I have some recent changes that I haven't tested at all yet, so I am not pushing the branch yet. But that could happen any time now (famous last words).

The editing commit messages and having git ask emacs(client) to do it (or telling git to use the already written message, as we used to do) or very distinct tasks but they are currently to strongly connected (and to some extend they also were with the old log-edit).

So this is not only an attempt to finally fix the client interaction but also to separate these concerns. This might make it easier to restore a client-less commit as an option (should we, after fixing the client approach, decide we still want that).

It will also allow using magit-interactive-rebase when using tramp (#591), something I probably would never have done if it weren't for this commit breakage.

And last but not least git-commit-mode itself should be a minor mode (deriving from with-editor-minor-mode) to allow the use of e.g. markdown-mode when editing commit messages, as requested in magit/git-modes#17.

@afrepues
Copy link
Contributor

I have the feeling that we are hitting on things that the VC developers have bumped into before, or at least is what I get from reading vc-git.el, vc-dispatcher.el and log-edit.el.

@tarsius do publish the branches, in your personal repository so we can help by testing and commenting, maybe even with code.

@tarsius
Copy link
Member

tarsius commented Sep 1, 2013

git-commit-mode prevents magit from running any other git operations in any repo

That's not really true. As I have said on #846:

It is not possible to do things that result in a call to magit-run* (e.g. push, pull) but [most?] things that one would actually want to do while composing a message are possible. E.g. the commit diff can be inspected and even be created if it doesn't exist yet, all while writing the message. Many other operations that might make sense are also possible - basically everything that ends up calling magit-cmd-output as opposed to magit-run* works.

Regardless I still intend to stop using magit-run-git-async in magit-commit.

@tarsius
Copy link
Member

tarsius commented Oct 20, 2013

I have just made the old commit mode available as a separate package. Until the new commit mode is functioning in all its glory, you might or might not want to use that.

The repository is a https://github.com/magit/magit-log-edit. It should also appear on Melpa soon.

@dabrahams
Copy link
Contributor

A quick 👍 for a less-modal commit mode, thanks

@tarsius
Copy link
Member

tarsius commented Dec 29, 2013

Four years and ~1100 issues later I have taken care of #24. That fixes this issue too :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

5 participants