Ticket #397 (closed defect: wontfix)

Opened 10 years ago

Last modified 9 years ago

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

Reported by: bnh Owned by: slavazanko
Priority: minor Milestone: 4.7.0-pre4
Component: mc-core Version: master
Keywords: i18n Cc:
Blocked By: Blocking:
Branch state: Votes for changeset: committed-master

Description

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.

Attachments

mouse-sort.patch (2.8 KB) - added by bnh 10 years ago.
Patch to screen.c to allow sorting on panel columns via mouse click
Q_.diff (3.3 KB) - added by dmartina 10 years ago.
Use context for indicator translations.

Change History

Changed 10 years ago by bnh

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

comment:1 Changed 10 years ago by bnh

  • 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

comment:2 Changed 10 years ago by andrew_b

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

comment:3 in reply to: ↑ description Changed 10 years ago by andrew_b

Replying to bnh:

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.

comment:4 Changed 10 years ago by angel_il

  • Milestone changed from 4.7 to 4.7.0-pre1

comment:5 Changed 10 years ago by angel_il

  • Milestone changed from 4.7.0-pre1 to 4.7

comment:6 Changed 10 years ago by angel_il

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

comment:7 Changed 10 years ago by slavazanko

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

reated branch 397_mouse_click_column_heading

initial changeset:dac1c1c21fcff330582c4d66cd13dcbd2fab45ac

review, please.

comment:8 Changed 10 years ago by slavazanko

  • Blocked By 212 added

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.

comment:9 Changed 10 years ago by slavazanko

  • severity changed from on review to on rework

comment:10 Changed 10 years ago by slavazanko

  • severity changed from on rework to on review
  • Blocked By 212 removed

See changeset:45c0931f997eeddb327a7defe9a8ffa48ae2c59e

Added function and key binding for toggle sort types.

For review, you may change mc.keymap:

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

Review, please.

comment:11 Changed 10 years ago by andrew_b

  • Milestone changed from 4.7.0-pre3 to 4.7.0-pre4

comment:12 Changed 10 years ago by slavazanko

Fix toogle sort after select sort type from dialog.
changeset:1c65f3bb72c77779cfa50630c7e707b43aa2be61

comment:13 Changed 10 years ago by slavazanko

Work complete.

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.

comment:14 Changed 10 years ago by iNode

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

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

comment:15 Changed 10 years ago by slavazanko

Now all fixed. See:

Review, please.

comment:16 Changed 10 years ago by iNode

  • Votes for changeset set to iNode

comment:17 Changed 10 years ago by slavazanko

  • Votes for changeset iNode deleted

new commits into branch:

Review.

comment:18 Changed 10 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:19 Changed 10 years ago by iNode

  • Votes for changeset changed from andrew_b to andrew_b iNode
  • severity changed from on review to approved

comment:20 Changed 10 years ago by slavazanko

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

comment:21 Changed 10 years ago by slavazanko

  • Status changed from testing to closed

comment:22 Changed 10 years ago by dmartina

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.

comment:23 follow-up: ↓ 29 Changed 10 years ago by 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.

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.

comment:24 Changed 10 years ago by slavazanko

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

Reopen because need to fix indicator of sort types.

comment:25 Changed 10 years ago by slavazanko

  • Status changed from reopened to accepted

comment:26 Changed 10 years ago by slavazanko

  • Keywords i18n added
  • severity changed from no branch to on review

Created branch 397_i18n_sort_indicator

Initial changeset:7088ee899ae8c1a85ae74b49ee7c3689c3aa7c95

Review, please.

comment:27 Changed 10 years ago by iNode

  • Votes for changeset set to iNode

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

comment:28 follow-up: ↓ 30 Changed 10 years ago by 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?

comment:29 in reply to: ↑ 23 Changed 10 years ago by dmartina

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!

comment:30 in reply to: ↑ 28 Changed 10 years ago by dmartina

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

comment:31 Changed 10 years ago by slavazanko

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

changeset: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).

comment:32 Changed 10 years ago by andrew_b

  • Votes for changeset changed from iNode to iNode andrew_b
  • severity changed from on review to approved

comment:33 follow-up: ↓ 36 Changed 10 years ago by slavazanko

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

comment:34 Changed 10 years ago by slavazanko

  • Status changed from testing to closed

comment:35 Changed 10 years ago by slyfox

  • 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

comment:36 in reply to: ↑ 33 ; follow-up: ↓ 37 Changed 10 years ago by slyfox

Replying to slavazanko:

merge changeset:11ec99633325b84c9601159101ba4c7072ae6f0d

Also, updated translations: de72a6f98f2b9c60d55c16e638b73bbd3b5328b7

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 критическая ошибка

comment:37 in reply to: ↑ 36 Changed 10 years ago by andrew_b

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

Replying to slyfox:
< {{{

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

Fixed directly in master. changeset:e2e549d4bc529e174548784d09e9af2e5623497d

comment:38 Changed 10 years ago by slavazanko

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

created branch 370_fix_init_struct

initial changeset:55fe1bb86e68d447818844025372888467619211

Review, please.

Sorry for inconsistence :(

comment:39 Changed 10 years ago by iNode

  • Votes for changeset set to iNode

comment:40 Changed 10 years ago by angel_il

  • Votes for changeset changed from iNode to iNode angel_il
  • severity changed from on review to approved

comment:41 Changed 10 years ago by slavazanko

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

comment:42 Changed 10 years ago by slavazanko

  • Status changed from testing to closed

comment:43 Changed 10 years ago by dmartina

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.

Changed 10 years ago by dmartina

Use context for indicator translations.

comment:44 follow-up: ↓ 45 Changed 10 years ago by 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?

comment:45 in reply to: ↑ 44 Changed 10 years ago by dmartina

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.

comment:46 Changed 10 years ago by dmartina

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.

comment:47 Changed 10 years ago by slavazanko

  • 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? :)

comment:48 Changed 10 years ago by slavazanko

  • Status changed from reopened to accepted

comment:49 Changed 10 years ago by slavazanko

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

I created tickets:

  • #1714 - Sort types: use context for indicator translations.
  • #1715 - Sort types: duplicate of i18n strings
  • #1716 - Adding of fully featured skin

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

comment:50 Changed 10 years ago by slavazanko

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.