Ticket #397 (closed defect: wontfix)

Opened 7 years ago

Last modified 7 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 7 years ago.
Patch to screen.c to allow sorting on panel columns via mouse click
Q_.diff (3.3 KB) - added by dmartina 7 years ago.
Use context for indicator translations.

Change History

Changed 7 years ago by bnh

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

comment:1 Changed 7 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 7 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 7 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 7 years ago by angel_il

  • Milestone changed from 4.7 to 4.7.0-pre1

comment:5 Changed 7 years ago by angel_il

  • Milestone changed from 4.7.0-pre1 to 4.7

comment:6 Changed 7 years ago by angel_il

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

comment:7 Changed 7 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 7 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 7 years ago by slavazanko

  • severity changed from on review to on rework

comment:10 Changed 7 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 7 years ago by andrew_b

  • Milestone changed from 4.7.0-pre3 to 4.7.0-pre4

comment:12 Changed 7 years ago by slavazanko

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

comment:13 Changed 7 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 7 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 7 years ago by slavazanko

Now all fixed. See:

Review, please.

comment:16 Changed 7 years ago by iNode

  • Votes for changeset set to iNode

comment:17 Changed 7 years ago by slavazanko

  • Votes for changeset iNode deleted

new commits into branch:

Review.

comment:18 Changed 7 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:19 Changed 7 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 7 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 7 years ago by slavazanko

  • Status changed from testing to closed

comment:22 Changed 7 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 7 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 7 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 7 years ago by slavazanko

  • Status changed from reopened to accepted

comment:26 Changed 7 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 7 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 7 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 7 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 7 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 7 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 7 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 7 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 7 years ago by slavazanko

  • Status changed from testing to closed

comment:35 Changed 7 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 7 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 7 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 7 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 7 years ago by iNode

  • Votes for changeset set to iNode

comment:40 Changed 7 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 7 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 7 years ago by slavazanko

  • Status changed from testing to closed

comment:43 Changed 7 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 7 years ago by dmartina

Use context for indicator translations.

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

  • Status changed from reopened to accepted

comment:49 Changed 7 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 7 years ago by slavazanko

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