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

[PATCH] Mouse click on column heading sorts panel on that column #397

Closed
mc-butler opened this issue Jun 9, 2009 · 52 comments
Closed

[PATCH] Mouse click on column heading sorts panel on that column #397

mc-butler opened this issue Jun 9, 2009 · 52 comments
Assignees
Labels
area: core Issues not related to a specific subsystem prio: low Minor problem or easily worked around
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/397
Reporter bnh (bnhunsaker@….com)
Keywords i18n

Attached patch that adds the following 2 enhancements:
1) Using the mouse to click on a panel column heading re-sorts

the panel on that column. If already sorted on that column,
the sort order is reversed.

2) added a button "." next to "v" that toggles the display

of hidden files.

Note

Original attachments:

  • mouse-sort.patch (raw) by bnh (bnhunsaker@….com) on Jun 9, 2009 at 21:00 UTC
  • Q_.diff (raw) by dmartina (dhmartina@….es) on Oct 12, 2009 at 9:34 UTC
@mc-butler
Copy link
Author

Changed by bnh (bnhunsaker@….com) on Jun 9, 2009 at 21:00 UTC

Patch to screen.c to allow sorting on panel columns via mouse click

@mc-butler
Copy link
Author

Changed by bnh (bnhunsaker@….com) on Jun 9, 2009 at 21:05 UTC (comment 1)

  • Summary changed from Mouse click on column heading sorts panel on that column to [PATCH] Mouse click on column heading sorts panel on that column

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 10, 2009 at 1:59 UTC (comment 2)

  • Priority changed from major to minor
  • Keywords mouse, column, sort deleted
  • Milestone changed from 4.6.3 to 4.7

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 10, 2009 at 5:55 UTC

Replying to bnh (#397):

2) added a button "." next to "v" that toggles the display

of hidden files.

It's a semi-solution. In addition to mouse, the shortcut should be provided.
The length of panel title (current directory) should be corrected also.

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Jun 17, 2009 at 11:38 UTC (comment 4)

  • Milestone changed from 4.7 to 4.7.0-pre1

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Jun 18, 2009 at 16:39 UTC (comment 5)

  • Milestone changed from 4.7.0-pre1 to 4.7

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Aug 4, 2009 at 7:42 UTC (comment 6)

  • Milestone changed from 4.7 to 4.7.0-pre3
  • Severity set to no branch

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Sep 18, 2009 at 14:01 UTC (comment 7)

  • Owner set to slavazanko
  • Severity changed from no branch to on review
  • Status changed from new to accepted

reated branch 397_mouse_click_column_heading

initial [dac1c1c21fcff330582c4d66cd13dcbd2fab45ac]

review, please.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Sep 22, 2009 at 8:03 UTC (comment 8)

  • Blocked by set to #212

It's a semi-solution. In addition to mouse, the shortcut should be provided.

In this case this branch wil continue to develop after megre #212 into master.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Sep 29, 2009 at 18:41 UTC (comment 9)

  • Severity changed from on review to on rework

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Sep 29, 2009 at 21:10 UTC (comment 10)

  • Severity changed from on rework to on review
  • Blocked by #212 deleted

See [45c0931f997eeddb327a7defe9a8ffa48ae2c59e]

Added function and key binding for toggle sort types.

For review, you may change mc.keymap:

[panel]
...
-PanelToggleSortOrder =
+PanelToggleSortOrder = <some-hotkey>

Review, please.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 30, 2009 at 4:31 UTC (comment 11)

  • Milestone changed from 4.7.0-pre3 to 4.7.0-pre4

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Sep 30, 2009 at 7:34 UTC (comment 12)

Fix toogle sort after select sort type from dialog.
[1c65f3bb72c77779cfa50630c7e707b43aa2be61]

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 1, 2009 at 15:00 UTC (comment 13)

Work complete.

  • [3ca63869bbca682555596e3c42fe3e697996d5a6] : Added support of mouse events
  • [4fb134450cf2e85c53c2fe71f904e8044b188f19] : Some fixies from Andrew Borodin
  • [b0e01b7458bb2d58c0994a491fbe4da3e34a5c26] : reorganization of panel fields
  • [ca5555fc70aefae84c675df9eff95b93e896f1b0] : Remove hardcoded array of field names from listmode.c
  • [b41243a8be4a6b41db36c9da90ba36d167e59af7] : Added event handlers for select or toggle of sort types. Key bindings not provide (user may bind hotkeys to events as well).

For example, you may change mc.keymap like:

[panel]
PanelSelectSortOrder= alt-w
PanelToggleSortOrderPrev=alt-e
PanelToggleSortOrderNext=alt-d

Feel free to review and vote.

After vote branch will have 'on hold' status until release out 4.7.0-pre3.

@mc-butler
Copy link
Author

Changed by iNode (@iNode) on Oct 1, 2009 at 16:21 UTC (comment 14)

PanelSelectSortOrder does not change sort order and sometimes show incorrect sort order.

PanelToggleSortOrderPrev and PanelToggleSortOrderNext works good (but order of changes looks strange).

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 1, 2009 at 19:13 UTC (comment 15)

Now all fixed. See:

  • [627496125636981e223613d822ecfd1e8b374564]: Remove unneded code
  • [1e51fff77ceb6464ec9b2de206b89505a822f4ae]: Fixed behavior of CK_PanelSelectSortOrder event
  • [b2c601d2dceb590600ba73a172df2307057a0195]: Remove code for reverse search direction when toggle search types.
  • [10f3a940b5517d3df3df8bfab31457b17adf2759]: Added keymaps for some sort types. Also added added 'PanelReverseSort' keymap.

Review, please.

@mc-butler
Copy link
Author

Changed by iNode (@iNode) on Oct 1, 2009 at 21:01 UTC (comment 16)

  • Votes set to iNode

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 2, 2009 at 20:52 UTC (comment 17)

  • Votes iNode deleted

new commits into branch:

  • [a26db6491c14bd95a15ef0732789bc86c8d77fcf]: Added reversing sort order ability if one keybind raised multiple times.
  • [0d185fa2b8d3df30663ab6240d8c79eede92f383]: Fixed segfault in 'Brief file list' listing mode
  • [640e67fc2cfbe11e58bd3ca11a06a489bf66cc61]: Added indicator for current sort type and sort direction
  • [1420e7d4bdc631711a9269bb884cce1cdf511ba5]: Cleanup of code. Remove unused panel_field_t::use_in_gui variable

Review.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 6, 2009 at 7:08 UTC (comment 18)

  • Votes set to andrew_b

@mc-butler
Copy link
Author

Changed by iNode (@iNode) on Oct 6, 2009 at 7:09 UTC (comment 19)

  • Votes changed from andrew_b to andrew_b iNode
  • Severity changed from on review to approved

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 6, 2009 at 7:14 UTC (comment 20)

  • Votes changed from andrew_b iNode to commited-master
  • Status changed from accepted to testing
  • Resolution set to fixed
  • Severity changed from approved to merged

merge [77896d5]

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 6, 2009 at 7:15 UTC (comment 21)

  • Status changed from testing to closed

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 7, 2009 at 22:03 UTC (comment 22)

I'm not sure if using the &Hotkey for the indicator in the top left corner is the best choice. Hotkey is choosen for it's uniqueness in dialog and sometimes it is not quite representative.
For example, I tkink 'A' is the proper indicator for ATime, but I can't make it hotkey as "tiempo de &Acceso" because I'm already using it in "&Aceptar" for "OK". I suppose there may be similar troubles in other languages. I would let translators decide, an issue some TRANSLATORS messages in .po files in order to warn them about this use and not using more than a single character.

Something else: in screen.c line 1329, panel_paint_sort_info()

hotkey[1] = '\0';

doesn't seem to me very UTF-8 safe.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 7, 2009 at 22:51 UTC (comment 23)

I'm not sure if using the &Hotkey for the indicator in the top left corner is the best choice.
Hm... Is you have alternative proposal? I think about this and I found variants:

  • show sort indicator as is in English (for example, show first char from field name)
  • show sort indicator as one translated char.

In second variant need to inform all translators about one symbol in *.po files (what this mean).

How better? Is someone have third variant?

Something else: in screen.c line 1329, panel_paint_sort_info()
hotkey[1] = '\0';
doesn't seem to me very UTF-8 safe.

All ok. in previous line:

1328         hotkey = g_strdup(panel->current_sort_field->id);

'id' will not transform into UTF-8 and never translated. This is id of field. For example, see line 436, 448 etc. In these lines 'id' - it's a first string constant.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 7, 2009 at 23:01 UTC (comment 24)

  • Votes commited-master deleted
  • Resolution fixed deleted
  • Type changed from enhancement to defect
  • Status changed from closed to reopened
  • Severity changed from merged to no branch

Reopen because need to fix indicator of sort types.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 7, 2009 at 23:01 UTC (comment 25)

  • Status changed from reopened to accepted

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 8, 2009 at 10:43 UTC (comment 26)

  • Keywords set to i18n
  • Severity changed from no branch to on review

Created branch 397_i18n_sort_indicator

Initial [7088ee899ae8c1a85ae74b49ee7c3689c3aa7c95]

Review, please.

@mc-butler
Copy link
Author

Changed by iNode (@iNode) on Oct 8, 2009 at 11:27 UTC (comment 27)

  • Votes set to iNode

It's ugly solution that violate DRY principle, and require
from translators read sources, but what's done is done.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 8, 2009 at 12:23 UTC (comment 28)

Changeset for rebase:6a3b517a955d07c44377b3626c9e362bd4fdc97d

dmartina, if this not hard for you: review too, please. Is branch contain good way?

iNode: is you have solution?

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 8, 2009 at 19:23 UTC (comment 23.29)

Replying to slavazanko:

I'm not sure if using the &Hotkey for the indicator in the top left corner is the best choice.
Hm... Is you have alternative proposal? I think about this and I found variants:

  • show sort indicator as is in English (for example, show first char from field name)
  • show sort indicator as one translated char.

I would vote for the second one.

In second variant need to inform all translators about one symbol in *.po files (what this mean).

See this in args.c. The comment gets into po files so that translators don't have to read source.
#. TRANSLATORS: don't translate keywords and names of colors
#: src/args.c:309
The comment has to be in the previous line, before the string marked for translation.

How better? Is someone have third variant?

Something else: in screen.c line 1329, panel_paint_sort_info()
hotkey[1] = '\0';
doesn't seem to me very UTF-8 safe.

All ok. in previous line:

1328         hotkey = g_strdup(panel->current_sort_field->id);

'id' will not transform into UTF-8 and never translated. This is id of field. For example, see line 436, 448 etc. In these lines 'id' - it's a first string constant.

I trust you!

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 8, 2009 at 19:28 UTC (comment 28.30)

Replying to slavazanko:

Changeset for rebase:6a3b517a955d07c44377b3626c9e362bd4fdc97d

dmartina, if this not hard for you: review too, please. Is branch contain good way?

iNode: is you have solution?

I have no time to compile and full check it now... I took a look at the patch and it seems OK

TRANSLATORS: this is indicator for 'Change time' sort mode

I would remark the one character length. Something as :
TRANSLATORS: one single character to represent 'Change time' sort mode

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 8, 2009 at 22:19 UTC (comment 31)

TRANSLATORS: one single character to represent 'Change time' sort mode

[d88182b5d99e1b2ab7cfec93591f288885527ba9]

All ok. in previous line:

I trust you!

This code now removed as deprecated. :)

See this in args.c. The comment gets into po files so that translators don't have to read source.
#. TRANSLATORS: don't translate keywords and names of colors

Yepp, this works as well in my patches too.

After merge branch into 'master', I'll update all *.po files directly into master (to avoid lot of possible conflicts in merge).

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 9, 2009 at 6:38 UTC (comment 32)

  • Votes changed from iNode to iNode andrew_b
  • Severity changed from on review to approved

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 9, 2009 at 6:52 UTC (comment 33)

  • Resolution set to fixed
  • Votes changed from iNode andrew_b to commited-master
  • Severity changed from approved to merged
  • Status changed from accepted to testing

merge [11ec996]

Also, updated translations: [de72a6f]

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 9, 2009 at 6:52 UTC (comment 34)

  • Status changed from testing to closed

@mc-butler
Copy link
Author

Changed by slyfox (@trofi) on Oct 10, 2009 at 9:47 UTC (comment 35)

  • Severity changed from merged to on rework

There is missing line in one of structs:

gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../../mc/src -I..  -DDATADIR=\""/home/slyfox/dev/git/mc-build/_mc-bin/share/mc/"\" -DLOCALEDIR=\""/home/slyfox/dev/git/mc-build/_mc-bin/share/locale"\" -DSAVERDIR=\""/home/slyfox/dev/git/mc-build/_mc-bin/libexec/mc"\" -DSYSCONFDIR=\""/home/slyfox/dev/git/mc-build/_mc-bin/etc/mc/"\"  -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -I../../mc  -g -O2 -Wall -MT screen.o -MD -MP -MF .deps/screen.Tpo -c -o screen.o ../../mc/src/screen.c
../../mc/src/screen.c:470: warning: initialization makes integer from pointer without a cast
../../mc/src/screen.c:470: error: initializer element is not computable at load time
../../mc/src/screen.c:470: error: (near initialization for 'panel_fields[4].use_in_user_format')
../../mc/src/screen.c:472: warning: initialization from incompatible pointer type

@mc-butler
Copy link
Author

Changed by slyfox (@trofi) on Oct 10, 2009 at 9:54 UTC (comment 33.36)

Replying to slavazanko:

merge [11ec996]

Also, updated translations: [de72a6f]

msgstr "Без сортировки"" 

Too many trailing ", it won't build:

cd ../../mc/po && rm -f ru.gmo && /usr/bin/gmsgfmt -c --statistics -o ru.gmo ru.po
ru.po:2684: символ конца строки встречен внутри строки
/usr/bin/gmsgfmt: найдена 1 критическая ошибка

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 11, 2009 at 5:02 UTC (comment 36.37)

  • Votes commited-master deleted
  • Status changed from closed to reopened
  • Resolution fixed deleted

Replying to slyfox:
< {{{

ru.po:2684: символ конца строки встречен внутри строки
/usr/bin/gmsgfmt: найдена 1 критическая ошибка
}}}

Fixed directly in master. [e2e549d]

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 6:44 UTC (comment 38)

  • Status changed from reopened to accepted
  • Severity changed from on rework to on review

created branch 370_fix_init_struct

initial [55fe1bb]

Review, please.

Sorry for inconsistence :(

@mc-butler
Copy link
Author

Changed by iNode (@iNode) on Oct 12, 2009 at 6:47 UTC (comment 39)

  • Votes set to iNode

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Oct 12, 2009 at 6:50 UTC (comment 40)

  • Severity changed from on review to approved
  • Votes changed from iNode to iNode angel_il

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 7:03 UTC (comment 41)

  • Votes changed from iNode angel_il to commited-master
  • Severity changed from approved to merged
  • Resolution set to fixed
  • Status changed from accepted to testing

merge [07b9ae3]

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 7:03 UTC (comment 42)

  • Status changed from testing to closed

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 12, 2009 at 9:10 UTC (comment 43)

If asking for one character translations may seem a bit weird, a context may be given. I'm attaching the patch. Anyway, I don't dislike the actual solution. Advice for translators should stay in any case.

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 12, 2009 at 9:34 UTC

Use context for indicator translations.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 12, 2009 at 9:52 UTC (comment 44)

In this ticket, I dislike the double i18n stuff. For example:

N_("Unsorted"), N_("&Unsorted"),
N_("Name"), N_("&Name"),
N_("Extension"), N_("&Extension"),

I wonder, is there any way to avoid that?

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 12, 2009 at 10:34 UTC (comment 44.45)

Replying to andrew_b:

In this ticket, I dislike the double i18n stuff. For example:

N_("Unsorted"), N_("&Unsorted"),
N_("Name"), N_("&Name"),
N_("Extension"), N_("&Extension"),

I wonder, is there any way to avoid that?

Trim any '&' after runtime translation. But keep in mind that these single word translations may cause trouble when used in different contexts (ie.: different shortcuts or even available length in a menu or in a dialog), so more use of the Q_ macro would be quite convenient.

@mc-butler
Copy link
Author

Changed by dmartina (dhmartina@….es) on Oct 12, 2009 at 12:01 UTC (comment 46)

I have tried to change the sort indicators to have up/down arrows instead of tildes and it looks really nice. ¿What would be the best choice? ¿System or user skins? I think it might at least be included as part of the double-lines skin.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 13:55 UTC (comment 47)

  • Status changed from closed to reopened
  • Resolution fixed deleted

I think it might at least be included as part of the double-lines skin.

Hm... Totally I agree with this. 'Standart' skin must be untouched (or must contain very simple skin for show on many systems as this possible). In opposite, 'double-lines' skin will contain enhanced skin as example of usage.

I think, need to rename 'double-lines' skin into 'featured'... or need to create new repo-inside file... what better? :)

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 13:58 UTC (comment 48)

  • Status changed from reopened to accepted

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 22:43 UTC (comment 49)

  • Resolution set to wontfix
  • Status changed from accepted to testing

I created tickets:

Therefore this ticket is closed.
Don't reopen, please :) Create new tickets instread.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Oct 12, 2009 at 22:44 UTC (comment 50)

  • Status changed from testing to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues not related to a specific subsystem prio: low Minor problem or easily worked around
Development

No branches or pull requests

2 participants