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

feat: Builtin Bcommit switch preview content #307

Closed
weilbith opened this issue Dec 2, 2020 · 6 comments · Fixed by #528
Closed

feat: Builtin Bcommit switch preview content #307

weilbith opened this issue Dec 2, 2020 · 6 comments · Fixed by #528

Comments

@weilbith
Copy link
Contributor

weilbith commented Dec 2, 2020

Hey guys,
just such amazing work going on here every day.
I'm just about to remove some old plugins that have been obsolete by Telescope. One of them is related to the git commit history of the current buffer. I would love to ask if it would be possible to alternate the preview with a key mapping between:

  • diff to current state
  • diff to it parent commit
  • plain file as it was/is for the selected commit

That would be incredibly useful. Thanks for any feedback! 🤗

@weilbith weilbith changed the title Feature Request - Bultin Bcommit switch preview mode Feature Request - Bultin Bcommit switch preview content Dec 2, 2020
@tjdevries
Copy link
Member

@Conni2461 might have some insights on this.

If you're interested, we could try and help you implement the features if you want :)

@Conni2461
Copy link
Member

I think that could be implemented pretty straight forward. I think the only thing that is missing is a interface to just pass in a new previewer and load this one. But that should be pretty easy too.

I am just not sure if we want it inside core. We had a discussion here #289 (comment) about providing a good enough default and than doing a telescope-git.nvim extension which includes more enhanced stuff with a lot more mappings/actions.

If tj says: Lets just include that in core for now, I am willing to provide help if you want to work on that feature.

Also we should probably do this for builtin.commits as well. If we do it.

Also also thanks for the kind words. :)

@weilbith
Copy link
Contributor Author

weilbith commented Dec 3, 2020

Alright. I must admit that so far I was mostly watching the project (and using ofc). But I have barely insights of the internals. I remember that there were many discussions and "refactorings" how do do things right/generic enough. So I'll have to take a closer look insight first, before I'm able to come up with a solution approach. I would share such with you before start to code.

I somewhere already read about the plans to start this dedicated git extension plugin. Was it already created? I agree that we should keep the main plugin simple and generic enough.

@Conni2461
Copy link
Member

I must admit that so far I was mostly watching the project

Great to hear :)
We had some refactorings for the previewer and i think we are in a good state right now but these things should not concern you if you are going to implement it.

I think i should just hint you to the correct place to look. The builtin is defined here lua/telescope/builtin/git.lua and will lead you toward the current previewer. I suggest just writing the new previewers for now and replacing them in the finder definition. After that we can talk about how to replace the previewer on key press.
Also do require('plenary.reload').reload_module('telescope'); before running the builtin. So you don't have to restart neovim. And if you already knew that even better :)

Currently there is no git extension because we don't have a way to inject mappings from an extension yet.

Also you can just join telescope gitter and we can talk more there.

@weilbith
Copy link
Contributor Author

weilbith commented Dec 3, 2020

I suggest just writing the new previewers for now and replacing them in the finder definition. After that we can talk about how to replace the previewer on key press.

I'm a little confused. The current previewer will not become obsolete. Rather two more are about to be added. So what is the point of replacing it know with one of the two others before figuring out how to alternate them? I mean sure I could start with that to get into it. I could first make them different pickers for the beginning and then merge them into one. Or did I misunderstood you.

Also do require('plenary.reload').reload_module('telescope'); before running the builtin. So you don't have to restart neovim. And if you already knew that even better :)

I knew it, but great hint anyways. Thanks. 🙏

Also you can just join telescope gitter and we can talk more there.

I'll probably join. Just have to actually work now. That's the disadvantage of working professionally with GitHub and using the same account. 😂

@Conni2461
Copy link
Member

Conni2461 commented Dec 3, 2020

I understood you don't wanna replace the previewer but its a good way to just write the new previewers and test it with just calling them where the current previewer is. After both new previewer work we can think of an interface how to switch the previewer and map it.

Just have to actually work now.

Take all the time you need

@kkharji kkharji changed the title Feature Request - Bultin Bcommit switch preview content feat: Builtin Bcommit switch preview content Dec 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants