---------- Forwarded message ----------
From: Cosmin Popescu <cosminpope...@members.fsf.org>
Date: Tue, Oct 15, 2013 at 8:37 PM
Subject: Re: MC Tabs
To: Egmont Koblinger <egm...@gmail.com>


Dear Egmont,

I will answer your observations one by one:

your patch desperately
lacks the "move this tab to the other panel" (or something similar)
option with menu entries and convenient shortcuts, or maybe ^U could
just swap two tabs but not two whole panels.  Or, tabs should not be
per-panel but common to the two, in each panel you could easily jump
to any of the tabs.

----------------------------------------------------------------------

I can certainly add another option for the ^U shortcut (to move all tabs or
only the current tab). I can also add a menu entry to move the current tab
on the other panel. On the other hand, I prefer to have the tabs
separately. If anyhow, somebody wishes to have them common for both panels
I can develop something like this (as an option) to be included in mc. But
for me, I don't think it's necessary.

==========================


Most of the Ctrl+Alt+letter combos are taken by my window manager, so
I'm back to entering Meta by pressing Escape.  Still, Esc followed by
C-Left or C-Right or C-g don't work for me, I'm not sure why.

------------------------------------------------------------------------

Since I wasn't sure that I know all the shortcuts of MC, I preferred not to
put any default shortcuts and expose my menu options to a keymap file. So
normally you should be able to assign any shortcut that your terminal
permits to any function that I've implemented.

===========================

The current UI trend (apart from a couple of legacy apps that had tab
support before it became widely used) is to place tabs on the top,
rather than the bottom.

--------------------------------------------------------------------------

I copied the krusader, which has by default the tabs on the bottom. But,
just like for krusader, I was thinking to add the possibility in the tabs
options to choose where the tab bar should be implemented. I choose to
begin with printing them down because it was easier to implement just by
following the existing code.

===========================


The "create tab" and such menu options operate on the currently active
panel, rather than on the Left/Right panel according to whose menu
entry is selected.  (E.g. Left panel is active, invoke Right->Create,
a new tab should be created on the right panel.)

---------------------------------------------------------------------------

I don't know why I was under the impression that the other options work the
same way. I've checked again now and you are right. I will correct this and
send you again another patch.

============================


Tabs should be selectable by a single mouse click (also scrolling
should be doable by mouse if too many tabs are open to fit).

-----------------------------------------------------------------------------

The truth is that I don't use the mouse with mc. This is why the tabs are
not responsive to the mouse. But again, if you would like to include it in
the official code repository I can also make it responsive to the mouse.

============================


Having non-ascii characters (don't forget about CJK either) cause
artifacts with highlighing the active tab (the end of the tab bar gets
the highlighed background too).

----------------------------------------------------------------------------

I think the issue there is that I used the strlen function. I will have a
look. I only checked with normal UTF-8 characters.

============================


One more thing: please use unified diffs, they are much more reable
and more portable to newer mc versions.

-----------------------------------------------------------------------------

The next patch I will send I will use unified diffs.


On Tue, Oct 15, 2013 at 8:37 PM, Egmont Koblinger <egm...@gmail.com> wrote:

> Hi Cosmin,
>
> This is a very interesting idea.  Please let me give feedback after 5
> minutes of using it :) Please note that I'm not an official mc
> developer, just an occasional contributor.
>
> I always wondered why mc has hardcodedly only 2 panels, not allowing
> more.  E.g. the screen could be divided in 2x2 parts and copy/move
> operations could have a direction (horizontal, vertical or diagonal).
> Your new feature approaches a similar goal.
>
> To accomplish the above mentioed goal, i.e. to be able to have 3
> panels and quickly copy between *any* two, your patch desperately
> lacks the "move this tab to the other panel" (or something similar)
> option with menu entries and convenient shortcuts, or maybe ^U could
> just swap two tabs but not two whole panels.  Or, tabs should not be
> per-panel but common to the two, in each panel you could easily jump
> to any of the tabs.
>
> Basic tab operations (next/prev/new/close) should also be something
> that has convenient short hotkeys by default, and those hotkeys are
> mentioned in the F9 dropdown.
>
> The current UI trend (apart from a couple of legacy apps that had tab
> support before it became widely used) is to place tabs on the top,
> rather than the bottom.
>
> The "create tab" and such menu options operate on the currently active
> panel, rather than on the Left/Right panel according to whose menu
> entry is selected.  (E.g. Left panel is active, invoke Right->Create,
> a new tab should be created on the right panel.)
>
> Tabs should be selectable by a single mouse click (also scrolling
> should be doable by mouse if too many tabs are open to fit).
>
> Having non-ascii characters (don't forget about CJK either) cause
> artifacts with highlighing the active tab (the end of the tab bar gets
> the highlighed background too).
>
> Sure there are a couple of more issues, but I kind of like your idea
> and would welcome such a feature.  Nice work for a start!
>
> One more thing: please use unified diffs, they are much more reable
> and more portable to newer mc versions.
>
>
> egmont
>
> On Mon, Oct 14, 2013 at 8:20 PM, Cosmin Popescu
> <cosminpope...@members.fsf.org> wrote:
> > Dear mc developers,
> >
> > Please find attached my contribution to mc: tabs like Total Commander
> has.
> >
> > I am a regular midnight commander user. I use it for all my file
> exploring
> > needs on several systems. I use it under Arch Linux and Ubuntu at home,
> > under Red Hat on various servers that I administer and under Cygwin on
> > Windows 7 at work.
> >
> > One of the things that was missing were the tabs and I hated all the
> time,
> > when I needed to copy some files from a location to several locations to
> > have to change the folder so many times.
> >
> > You will find an archive containing several patches that will add tabs
> to MC
> > and a keymap file to map some shortcuts for the tabs. Another message
> will
> > follow with some screen shots.
> >
> > Inside the archive there is an executable file called apply-patch that
> will
> > apply all the pathes on the required files. To install the patch, just
> > un-archive the patch.tar.gz inside the root folder of mc-4.8.10 archive,
> cd
> > to patch and run ./apply-patch.
> >
> > I've implemented the following:
> >
> > * Create tab (creates a new tab)
> > * Close tab (closes an existing tab, if the tab is not the only one)
> > * Rename tab (changes the title of the current tab)
> > * Next tab (changes the current tab)
> > * Previous tab (changes the current tab)
> > * Go to tab (displays a list of tabs from which the user can select one
> to
> > switch to)
> > * Tabs options (some tabs options)
> > * Tabs sessions (the current opened tabs can be saved into sessions that
> can
> > be later restored).
> >
> > In the tabs options there is an option to restore the last tabs session.
> > Please note that if this is checked, the current opened folder in the
> > current panel will not be the folder from which mc is launched, but the
> > folder that was last current when mc was closed.
> >
> > From a technical point of view, the tabs are a simple list of structures
> > containing a name (which can be NULL in which case the name of the
> current
> > tab is the name of the current folder) and a path (a vfs_path_t
> structure)
> > containing the current path of the tab. Whenever a tab is created, a new
> tab
> > structure is added in the list. When a tab is changing the mc will do a
> > simple cd by calling the do_cd function. I think that the implementation
> is
> > pretty much straight forward.
> >
> > The only thing a little bit complex might be the display_info function.
> That
> > function will determine if all the tabs can be displayed, if not will
> > calculate which can be displayed starting from the current tab to the
> left
> > and then to the right if there is still space.
> >
> > All my code is added between #ifdef WITH_TABS and #endif directives. The
> > WITH_TABS can be set at configure time. By default the tabs will be
> compiled
> > in, but can be deactivated by configure with "--disable-tabs" option.
> >
> > I also added a small implementation for "string_file_ext" function. I
> needed
> > it for my day to day work. It is added between the #ifdef WITH_EXT and
> > #endif directives. The WITH_EXT is defined in the src/filemanager/panel.c
> > file. If you don't want to use it, you can undefine the WITH_EXT.
> >
> > Also, please note that in the src/filemanager/filegui.c you have a small
> bug
> > at the line 288. The closing bracket of the function is inside the #ifdef
> > directive, while the opening one is outside. The program will of course
> fail
> > to compile under cygwin, so I've corrected it.
> >
> > In the src/filemanager/mountlist.c, on the line 245 you are using the
> > _GL_UNUSED macro. This will also fail to compile on my version of cygwin.
> > Although I know that probably I have to add a dev package to have the
> macro
> > defined, I don't think that it should be the case to do that just to
> avoid a
> > warning (to add a dependency). I would do that with something like #ifdef
> > _CYGWIN_ directive, but since this might be because of my installation of
> > cygwin, I didn't modify it in the patch that I've sent you.
> >
> > The patch is applied against the mc-4.8.10 downloaded from here:
> > http://ftp.midnight-commander.org/mc-4.8.10.tar.xz
> >
> > I've compiled the mc-4.8.10 with tabs on the following platforms:
> >
> > * Arch Linux
> > * Red Hat 5
> > * Ubuntu
> > * Cygwin (win 7)
> >
> > Please let me know if you would like to include the tabs in your main
> source
> > repository. If not, do you have something against me posting the patch on
> > sourceforce and github?
> >
> > Thank you very much for maintaining MC.
> >
> > Regards,
> > Cosmin Popescu.
> >
> > _______________________________________________
> > mc-devel mailing list
> > https://mail.gnome.org/mailman/listinfo/mc-devel
> >
>
_______________________________________________
mc-devel mailing list
https://mail.gnome.org/mailman/listinfo/mc-devel

Reply via email to