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

Use the "next" version of Magit #1941

Closed
Josh-Tilles opened this issue Jun 10, 2015 · 19 comments
Closed

Use the "next" version of Magit #1941

Josh-Tilles opened this issue Jun 10, 2015 · 19 comments
Labels

Comments

@Josh-Tilles
Copy link
Contributor

This isn't necessarily urgent; I just wanted to make sure it was on y'all's radar since Magit's maintainer has started to encourage maintainers of third-party tools to try the next branch out. (source: magit/magit#1645 (comment))

@syl20bnr
Copy link
Owner

Thanks for mentioning this, I'll see if I can quickly implement some feature toggle but if it is not distributed via an elpa repository we will have to use extensions, I may exceptionally introduce a temporary submodule in order to help getting feedback on next magit.

@syl20bnr syl20bnr added the Git label Jun 11, 2015
@tko
Copy link
Contributor

tko commented Jun 21, 2015

I tried switching to extensions and noticed that the way keybindings are defined has changed notably. There's something like 14 magit-TYPE-section-maps that appear to override any magit-mode and magit-status-mode bindings such that while j works vim-style k is bound to magit-discard-item rather than moving to next line, and I can't figure out how to change that.

@cpaulik
Copy link
Contributor

cpaulik commented Jun 21, 2015

Without looking at the code I would guess that the new mode-maps have to be evilified like in the current magit layer https://github.com/syl20bnr/spacemacs/blob/develop/contrib/!source-control/git/packages.el#L132-L175

@cpaulik
Copy link
Contributor

cpaulik commented Jun 21, 2015

Magit 2.1.0 will be released on the 1st of July (see here), so we should ideally make sure that this works. Otherwise I suspect a lot of issues about magit not working correctly.

@tko Do you already have a branch somewhere testing the implementation? I could help if you like.
@syl20bnr any thoughts about making this transition as smooth as possible?

@syl20bnr
Copy link
Owner

Make an extension with exceptionally a submodule pointing on magit2.
Call the extension package magit2 and add a layer variable to switch
between the two magit packages by playing with git-excluded-packages.
From there we can tweak the magit2 configuration and make it work with
spacemacs.

A few days before the release of magit2 we migrate the magit2 config to
packages.el with a condition on the magit internal version number.

A couple of weeks later we will be able to remove the magit1 specific
configuration.

Le dimanche 21 juin 2015, Christoph Paulik notifications@github.com a
écrit :

Magit 2.1.0 will be released on the 1st of July (see here
magit/magit#1645), so we should ideally make
sure that this works. Otherwise I suspect a lot of issues about magit not
working correctly.

@tko https://github.com/tko Do you already have a branch somewhere
testing the implementation? I could help if you like.
@syl20bnr https://github.com/syl20bnr any thoughts about making this
transition as smooth as possible?


Reply to this email directly or view it on GitHub
#1941 (comment)
.

-syl20bnr-

@tko
Copy link
Contributor

tko commented Jun 21, 2015

@cpaulik
Copy link
Contributor

cpaulik commented Jun 21, 2015

@tko OK I see, the evilify macro does not work on all the sparse keymaps that are defined for the sections. I was able to override the bindings by setting the key specifically for the mode but this seems very tedious and would break holy-mode

@syl20bnr I think this requires your expertise.

@cpaulik
Copy link
Contributor

cpaulik commented Jun 21, 2015

I've pushed my branch magit-next here. I can make a PR to you @tko so we have the latest status there but it is probably better to start fresh from develop since I had to add back in the things that you removed in packages.el

Basically I've added a variable magit-use-next that chooses the magit in the extension if set to t

@tko
Copy link
Contributor

tko commented Jun 21, 2015

@cpaulik I think you should probably just squash the patches together off of develop. I just threw some things together, I don't see myself finishing it :-/

@kvaneesh
Copy link

Can we get magit as a separate layer ?. Is it possible for one layer to include others. if yes, we can include magit layer to be part of larger git layer.

@cpaulik
Copy link
Contributor

cpaulik commented Jun 22, 2015

@kvaneesh I'm not sure that is makes sense to put magit into it's own layer since it is the core part of the git integration in emacs and spacemacs. If we want to enable/disable certain features of the git layer we can always do so using the layer variables.

@syl20bnr
Copy link
Owner

Thank you @tko and @cpaulik for starting to look at it.

I pushed magit-next submodule in develop, can be used by setting the layer variable git-use-magit-next to t.

Major issues for now:

  • keymap use the text property keymap on the sections (the highest priority possible for a key map), evil-define-key seems not to be aware of it. May require a patch in evil.
  • Action popups require to be in insert mode (we may be able to fix it easily by just switch to insert mode in those buffers).

@syl20bnr
Copy link
Owner

I'm currently working on the support for magit 2.1 and I plan to release the version 103 at the same time as the new magit (more or less).

Some update about the status:

  • I pushed some code to evilify the magit sections, due to the usage of the keymap text property I was forced to use some remap black magic in order to keep a clean support for Evilified and Emacs states (and the ability to switch dynamically the editing style), related commit: 42dc33a
  • Next step is to update the classical mode keymaps, a lot of commands have changed.
  • Last thing to support (I hope) is the commands popup buffer which need to be in insert state.

@syl20bnr
Copy link
Owner

I worked a lot on magit keymaps today and I added new functions to automatically evilify a map. It handled the translation of shadowed bindings described in the conventions and even handle "sub-keymap" (like j to jump between sections in magit). It also supports changing dynamically the editing style which was the difficult part.

It almost works, actually it works with all the keymaps except one: magit-mode-map. This is due to the fact that it is created with make-keymap instead of make-sparse-keymap like the other keymaps. If I change the creation to make-sparse-keymap then everything seems to work fine in magit 2.1!

I have still to work on the magit-popup which defaults to normal mode.

@kvaneesh
Copy link

@syl20bnr if magit can work with make-sparse-keymap, that can be a patch to magit upstream isn't it ?

@cpaulik
Copy link
Contributor

cpaulik commented Jun 26, 2015

Thanks @syl20bnr. I've just been testing it a little bit and I got the following error pressing K on a staged item.

Discard staged changes in .gitignore? (y or n) y
let*: Wrong type argument: symbolp, (lambda nil (interactive) (call-interactively (quote magit-discard)))

@cpaulik
Copy link
Contributor

cpaulik commented Jun 29, 2015

@syl20bnr Is there a reason against adding magit-popup-mode to evil-insert-state-modes. I've been using this the last 20 minutes and have not yet encountered problems.

The issue mentioned in my previous post might be a magit problem, I'll have to check. EDIT: no it works in holy-mode

@syl20bnr
Copy link
Owner

syl20bnr commented Jul 1, 2015

Sorry, I was working on the a new function to automatically evilify a map
according to the convention rules. This function is now ready and applied
to magit. I added some other tweaks like the magit-popup-mode (I used emacs
state instead of insert state since it works better with evil-escape).

I will update the bindings for the new interactive rebase and commit modes.

-syl20bnr-

On Mon, Jun 29, 2015 at 6:04 PM, Christoph Paulik notifications@github.com
wrote:

@syl20bnr https://github.com/syl20bnr Is there a reason against adding
magit-popup-mode to evil-insert-state-modes. I've been using this the
last 20 minutes and have not yet encountered problems.

The issue mentioned in my previous post might be a magit problem, I'll
have to check.


Reply to this email directly or view it on GitHub
#1941 (comment)
.

@syl20bnr
Copy link
Owner

syl20bnr commented Jul 1, 2015

I finished the support of magit next, you should be able to test it on develop with the following layer variable git-use-magit-next.

Closing this as magit next is not available, please open new issues if you find bugs.

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

No branches or pull requests

5 participants