Undo tab close #1668

etrunko · 2016-04-01T19:46:45Z

I always find it annoying when I close a channel tab by mistake and then have to join it again manually. Especially when the channel you just closed has long or weird name.

This request implements the action of undo tab close, as found in most web browsers today, by pressing CTRL+Shift+T.

The first patch fixes a small issue with key event handling. I think no one else had this problem before because there are no accelerators with the shift key.

The second one implements the feature for Gtk gui and it is a _draft_. The goal is to have a stack of recently closed channels, and not be restricted by only one channel. I would appreciate some advise of the best place to store such stack, if in a global variable or as a member of a data struct.

Comments appreciated.

TingPing · 2016-04-01T19:54:46Z

I'm not sure the case change is the right thing to do..

TingPing · 2016-04-01T19:56:46Z

src/fe-gtk/maingui.c

+ {
+ _last_chan.server = sess->server;
+ memcpy(_last_chan.channel, sess->channel, CHANLEN);
+ memcpy(_last_chan.key, sess->channelkey, 64);


g_strlcpy()

etrunko · 2016-04-01T19:59:00Z

@TingPing I tried without success to match Ctrl+Shift+T. The event will always have the code of the uppercase letter, while the stored value will always be lowercase. What do you suggest?

TingPing · 2016-04-01T19:59:07Z

src/fe-gtk/maingui.c

+ server *server;
+ char channel[CHANLEN];
+ char key[64];
+} _last_chan = { 0 };


I think it makes more sense to use a GQueue and store a few previous channels rather than a single one. Also don't start variables with _.

Yes, that is what I intended to do, as I wrote in the description of the pull req. I need some advice of where it should be stored.

Probably not in the maingui. I don't really know the ideal spot but for example the lastact feature in fkeys.c shoved a list in hexchat.c, don't know how awful that location is too.

TingPing · 2016-04-01T20:03:23Z

What do you suggest?

Will have to look into it, just doesn't sound correct, keys are case sensitive in my mind.

Arnavion · 2016-04-01T20:09:48Z

(Note: Haven't actually tested your code, and don't have HC in front of me to test atm.)

By ignoring the case completely, aren't you allowing the reverse to happen? i.e. CtrlShiftT will end up triggering shortcuts defined for CtrlT ?

I think instead of ignoring completely you want to check if the keyval is uppercase, then see if there's a corresponding shortcut for Shiftcorresponding lowercase and trigger it, else ignore it.

TingPing · 2016-04-01T20:10:56Z

BTW I wrote this a while ago (just realized it doesn't handle keys :P): https://github.com/TingPing/plugins/blob/master/HexChat/undoclose.py

etrunko · 2016-04-05T11:16:53Z

@Arnavion I have tested it here, it will only work for Ctrl+Shift+T. I guess you wanted to check if this change would mess with new tab when user has Caps-lock turned on right?

etrunko · 2016-04-05T11:18:55Z

I have updated patch 2 implementing it as a GQueue as suggested. Still missing the right place to store the pointer to that queue. It will leak some memory when you close the application with channel information stored.

etrunko · 2016-04-06T12:17:53Z

Updated patch to to avoid adding channels to the queue while hexchat is quitting.

etrunko · 2016-04-08T13:44:57Z

Another update, use g_new/g_free instead of calloc/free.

I am thinking about having this list global, in hexchat.c. Does this solution have a chance to go in? What about the fix for accelerators?

etrunko · 2016-05-11T22:27:30Z

New version of patch, the only difference is that we now clear closed channels when hexchat is quitting. I tried making the queue global, but did not like the result. Take a look https://gist.github.com/etrunko/a2dafb25c0ca49175c2b57b6f7011de9

I am much happier with current version, and I think it is ready for inclusion, comments appreciated.

etrunko · 2016-05-25T14:28:06Z

ping?!

etrunko · 2016-06-08T13:20:31Z

Hi @TingPing @Arnavion, any additional comments?

abnerf · 2016-06-08T13:31:39Z

+1

etrunko · 2016-07-13T12:41:50Z

Hi, another rebase of the code is up. Please take a look. It has been long time since any project developers replied to this pull request. I don't see any difficulties in having these patches merged, as the code is not intrusive, and I have been using it since I started working on it, but it is getting kind of painful to keep it out of the tree. As it happens with many other different open source projects out there, this kind of attitude unresponsiveness helps keeping people away from contributing and eventually using the project, unfortunately.

Arnavion · 2016-07-14T01:01:46Z

Since you're disallowing reopening a channel tab if its server tab has been closed (since you check is_server()), why not scope the queue to the server object?

Note that TingPing's script that he linked above does allow it. I think doing it the same way is the better approach, i.e., store just network + channel + key in the queue, and connect to the network if an existing connection is unavailable.

Arnavion · 2016-07-14T01:03:43Z

src/fe-gtk/maingui.c

+ return;
+
+ chan = g_queue_pop_head(&closed_channels);
+ if (is_server (chan->server))


It's a little suboptimal to silently ignore the undo command if this is false. The user will have no idea why nothing happened, and if they press it again expecting it didn't go through the first time, it might reopen a completely different tab.

etrunko · 2017-01-10T19:20:52Z

Hello there, I had not touched this until recently when I upgraded my distro and noticed this feature did not work anymore.

@Arnavion regarding scoping of the queue, I tried to do it with this new patch, but I lose the global behavior. https://gist.github.com/etrunko/c6a38e5b84227e06e092a16e7df5fe67

For instance if I have more than one server connected, and close a channel in one server, then focus on another channel of a different server, it will not restore the closed tab. I need to focus again on that other server and hit Ctrl+Shift+Tab to get that channel back open.

My idea is to reopen the last channel closed, independently of which server is focused, so I think it makes more sense to keep it global, but if you don't like the static declaration, I could easily put it under another struct, not sure which one though. Because there is no such thing as a maingui struct, I thought it would not make sense to do it like that.

Finally as for making this code work as the plugin one, I will take a look on the python bindings for the "command" function to see what it does.

etrunko · 2017-11-08T13:34:01Z

Updated branch with a commit adding shortcut for tab closing. Recent versions have disabled it somehow.

TingPing · 2017-11-09T02:04:01Z

They can already run the command /close I don't think it needs a new keyboard action.

etrunko · 2017-11-09T13:24:41Z

Well, for me it is something intrinsic to any GUI application which has tabs, such as web browsers, terminal emulators and chat applications. All of them support Ctrl+w and Crtl+Shift+t shortcuts, and that is really what this is all about, shortcuts. /close and other commands are really for people who know how to use IRC, and the user interface should provide the ways for people who are not aware those commands. For instance, I can easily change my nickname with /nick, but it is also possible to do it by clicking on the button on the main window. Hexchat itself supports Ctrl+t for a new tab, so I think it makes perfect sense to provide shortcuts for closing and "unclosing" tabs.

I wonder what really holds this change to be merged as is, and as I had already asked before, please provide me with pointers about what should be improved/changed to get this code in, so I can get rid of maintaining a separate branch and building manually.

If a new accelerator that involves the shift + another key (such as "<Shift>A") is added, key press event will never match that accelerator, because the key value will be the one corresponding capital letter, while the stored value will be the lowercase letter, as a result of the call to gkt_accelerator_parse(). Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

In recent versions the Ctrl+w shortcut for closing tabs has stopped working, probably due to some changes in Gtk+ (currently running fedora 26). So we add the shortcut back again. Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

etrunko force-pushed the undo_tab_close branch 2 times, most recently from de7f198 to fac4114 Compare April 1, 2016 19:48

TingPing reviewed Apr 1, 2016
View reviewed changes

etrunko force-pushed the undo_tab_close branch from fac4114 to 0459b12 Compare April 5, 2016 11:17

etrunko force-pushed the undo_tab_close branch from 0459b12 to 9e91b09 Compare April 6, 2016 12:17

etrunko force-pushed the undo_tab_close branch from 9e91b09 to e78671a Compare April 8, 2016 13:42

etrunko force-pushed the undo_tab_close branch from e78671a to 9c9953b Compare May 11, 2016 22:24

etrunko changed the title ~~[RFC] Undo tab close~~ Undo tab close May 12, 2016

etrunko force-pushed the undo_tab_close branch 2 times, most recently from b0da17a to c639c4e Compare May 25, 2016 14:42

etrunko force-pushed the undo_tab_close branch from c639c4e to 09e49b3 Compare July 13, 2016 12:33

Arnavion reviewed Jul 14, 2016
View reviewed changes

TingPing added the needs-work label Oct 8, 2016

etrunko force-pushed the undo_tab_close branch from 934e5e1 to 973ece0 Compare November 7, 2017 21:11

etrunko force-pushed the undo_tab_close branch from 973ece0 to 1bf0822 Compare March 8, 2018 14:23

etrunko added 4 commits April 11, 2018 18:53

Implement undo tab close

007299b

Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

Add shortcut for closing tab

eea4f7c

In recent versions the Ctrl+w shortcut for closing tabs has stopped working, probably due to some changes in Gtk+ (currently running fedora 26). So we add the shortcut back again. Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

Add Ctrl+l shortcut for Clear text

151e808

Signed-off-by: Eduardo Lima (Etrunko) <[email protected]>

etrunko force-pushed the undo_tab_close branch from 1bf0822 to 151e808 Compare April 11, 2018 21:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Undo tab close #1668

Undo tab close #1668

etrunko commented Apr 1, 2016

TingPing commented Apr 1, 2016

TingPing Apr 1, 2016

etrunko Apr 1, 2016

etrunko commented Apr 1, 2016

TingPing Apr 1, 2016

etrunko Apr 1, 2016

TingPing Apr 1, 2016

TingPing commented Apr 1, 2016

Arnavion commented Apr 1, 2016

TingPing commented Apr 1, 2016

etrunko commented Apr 5, 2016

etrunko commented Apr 5, 2016

etrunko commented Apr 6, 2016

etrunko commented Apr 8, 2016

etrunko commented May 11, 2016 •

edited

etrunko commented May 25, 2016

etrunko commented Jun 8, 2016

abnerf commented Jun 8, 2016

etrunko commented Jul 13, 2016

Arnavion commented Jul 14, 2016

Arnavion Jul 14, 2016

etrunko commented Jan 10, 2017

etrunko commented Nov 8, 2017

TingPing commented Nov 9, 2017

etrunko commented Nov 9, 2017

Undo tab close #1668

Are you sure you want to change the base?

Undo tab close #1668

Conversation

etrunko commented Apr 1, 2016

TingPing commented Apr 1, 2016

TingPing Apr 1, 2016

Choose a reason for hiding this comment

etrunko Apr 1, 2016

Choose a reason for hiding this comment

etrunko commented Apr 1, 2016

TingPing Apr 1, 2016

Choose a reason for hiding this comment

etrunko Apr 1, 2016

Choose a reason for hiding this comment

TingPing Apr 1, 2016

Choose a reason for hiding this comment

TingPing commented Apr 1, 2016

Arnavion commented Apr 1, 2016

TingPing commented Apr 1, 2016

etrunko commented Apr 5, 2016

etrunko commented Apr 5, 2016

etrunko commented Apr 6, 2016

etrunko commented Apr 8, 2016

etrunko commented May 11, 2016 • edited

etrunko commented May 25, 2016

etrunko commented Jun 8, 2016

abnerf commented Jun 8, 2016

etrunko commented Jul 13, 2016

Arnavion commented Jul 14, 2016

Arnavion Jul 14, 2016

Choose a reason for hiding this comment

etrunko commented Jan 10, 2017

etrunko commented Nov 8, 2017

TingPing commented Nov 9, 2017

etrunko commented Nov 9, 2017

etrunko commented May 11, 2016 •

edited