tmux-plugins / tmux-copycat

A plugin that enhances tmux search
MIT License
1.1k stars 63 forks source link

Support Tmux 2.4 mappings #119

Closed thalesmello closed 6 years ago

thalesmello commented 6 years ago

@bruno-

I've finally manage to convert tmux-copycat to support both Tmux 2.4, whereas keeping Tmux 2.3 support.

Could you take a look, please?

thalesmello commented 6 years ago

Addresses #118 and #109

bruno- commented 6 years ago

Hey,

moving forward, do you think we need to support tmux 2.3 and below? I'd tag current master, drop support for 2.3 and clearly state that we're 2.4 only (maybe instructions what to do to make it work for 2.3).

Also, does this work with tmux 2.5?

marcvs commented 6 years ago

I was trying with tmux 2.6, no success. prefix + c-f + entering one or two characters drops me back into my bash.

bruno- commented 6 years ago

From what I know support for tmux 2.4 was discontinued/dropped by maintainer after just a couple weeks because that version had some bugs. Frankly I'd be ok so skip 2.4 altogether and focus on 2.5, 2.6 and up.

thalesmello commented 6 years ago

@marcvs I just pushed a version that is supposed to be working. Would you be kind to test the new version, please?

@bruno- The code successfully runs with tmux 2.5 in my machine. I've taken the code that checks the version from the tmux yank project.

I think a lot of people use this plugins with old tmux versions. Therefore, my opinion is that we should keep on supporting pre-2.4 tmux versions.

The tests are failing, but plugin appears to be working correctly. Any idea on how to debug what might be wrong with the tests, @bruno- ?

thalesmello commented 6 years ago

@bruno- I noticed there were version specific setup files in the tests.

Adapting those still wasn't enough to make the tests work. Could you take a look?

marcvs commented 6 years ago

Yep: It works. Tested the key combinations in Readme.md, Version tested is tmux-2.6-rc3 as shipped in debian/testing.

thalesmello commented 6 years ago

@bruno- I've made all the tests currently pass when running using the ./run-tests script.

I think the next step would be to adapt the tests to make them work on travis. Any idea why they might not be working?

thalesmello commented 6 years ago

@bruno- I've finally made all tests pass in Travis. I don't think there is any work left in this PR to do. I'm waiting on your feedback.

bruno- commented 6 years ago

@thalesmello thank you very much for working on this! I checked out your branch locally and will test it for a day. I'm on tmux 2.6.

thalesmello commented 6 years ago

@bruno- Any updates?

bruno- commented 6 years ago

@thalesmello thank you for the patience. I've been testing copycat for the last 10 days or so and it's been ok.

The only big thing that bothers me is support for versions below tmux 2.4. It seems to be causing bloat (and slowness) in the code and tests. Can we drop that?

thalesmello commented 6 years ago

@bruno- I still think it's important to maintain backwards compatibility, for those situations you want to use the plugin with an older tmux version that might already come installed with a GNU/Linux distribution.

In order to make the problem maintaining different versions less of a burden, I've pushed an update that splits the calls the different actions in a polymorphic manner, such that version specific commands become less become easier to read through. In summary, I've replaced all the if-elses by a single if-else that dynamically loads the correct file for that tmux version.

Travis is only testing Tmux 2.4, as I couldn't find an easy way to run both 2.4 and legacy tests. It's taking longer because I've increased the sleep time quite a bunch, as the I was getting intermittent tests. Increasing the sleep time solved the problem.

Take a look and tell me what you find. If you think it's still not okay to support both versions of Tmux, give me a heads up so that I can keep a local version for my personal use, and I can adapt the current pull request to support 2.4 only.

If you need contributors to the project, I'd be glad to help.

Sincerely, Thales Mello.

thalesmello commented 6 years ago

@bruno- Nevermind.

I couldn't make that version work make the tests pass.

I just pushed a version with only 2.4 support.

But still, how are we going to make it convenient for people who use legacy tmux versions to use the compatible version?

TPM appears to not support branches or tags, from what I could find. Therefore, maybe we could make a github.com/tmux-plugins/tmux-copycat-legacy fork available?

thalesmello commented 6 years ago

@bruno- Did you have a chance to look at my new commits?

bruno- commented 6 years ago

@thalesmello thank you so much for working on this and having patience.

bruno- commented 6 years ago

Hi @thalesmello

I've invited you as a collaborator to this project! Welcome 🎉

Regarding the support for tmux 2.3: I added instructions for the users how they can do this link. Overall I think it's best and easiest for everyone (us and the users) if they upgrade to latest tmux version.

john-kurkowski commented 6 years ago

Addresses #118 and #109

Can we close those 2 now?

thalesmello commented 6 years ago

Yes. Those can be closed already.

On Sun, Nov 19, 2017, 20:54 John Kurkowski notifications@github.com wrote:

Addresses #118 https://github.com/tmux-plugins/tmux-copycat/issues/118 and #109 https://github.com/tmux-plugins/tmux-copycat/issues/109

Can we close those 2 now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tmux-plugins/tmux-copycat/pull/119#issuecomment-345557201, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEz4g1K_2rCC0eNNASXJdiL45NXMt3Yks5s4LGYgaJpZM4PqtYO .

oblitum commented 6 years ago

It seems this issue as mentioned at #109 is still present in this patch, see #121.