Skip to content

Conversation

@milkias17
Copy link
Contributor

mentioned in #550

@milkias17
Copy link
Contributor Author

@ten3roberts Any problems with the implementation😅?

@CKolkey CKolkey requested a review from ten3roberts July 16, 2023 21:00
Copy link
Member

@ten3roberts ten3roberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this works by not replacing the table reference, but rather works and mutates the existing reference?

@milkias17
Copy link
Contributor Author

Yup exactly. We are basically using the exact same method that diffview.nvim uses to merge the default configuration with the users configuration.

@milkias17 milkias17 force-pushed the diffview-mappings branch from 6ffd00f to 96838a4 Compare July 17, 2023 09:04
@milkias17 milkias17 force-pushed the diffview-mappings branch from 96838a4 to 3a2eec4 Compare July 17, 2023 09:05
@ten3roberts
Copy link
Member

Is this all done or is there anything more you want to get in?

Thanks for your help, I'm looking forward to having neogit invoked diffview be consistent with the user invoked diffview 🚀

@milkias17
Copy link
Contributor Author

Yea interms of repsecting user's configuration that's about it. However the thing mentioned about having a keybinding to invoke the commit popup within the diffview buffer, should I add that or?

@ten3roberts
Copy link
Member

Perhaps that feature would be best under a config option, since some people may not want their difftview keybindings to be different and inconsistent. There is an in-progress PR which modifies the config file, so if we do this it would be best as another PR after that one.

@ten3roberts ten3roberts merged commit 10eaae0 into NeogitOrg:master Jul 18, 2023
@milkias17 milkias17 deleted the diffview-mappings branch July 18, 2023 18:33
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.

2 participants