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

Modernize user menu #4571

Open
mc-butler opened this issue Aug 23, 2024 · 40 comments
Open

Modernize user menu #4571

mc-butler opened this issue Aug 23, 2024 · 40 comments
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4571
Reporter eugenesan (eugenesan@….com)

I've been working on modernizing the default user menu and decided to share it here.

First patch changes the default spacing in menu items.
The second patch:

  1. Removes archaic news and latex related items
  2. Reworks tarball options to support bare tarball, improve compressed tarballs
  3. Add support for regular archives (supported by 7z, which is most of them)
  4. Add support for compressing/un-compressing of more variants
  5. Add compression conversion
  6. Add hashing, git, vidir, ncdu, tail and mcdiff options

Feel free to adjust if I removed something people actually.

P.S.
From my previous experience using this ticket tracker, I never get notifications.
Do I need to do anything special to enable mail notifications for tickets?

P.P.S.
I chose mc-skin as a component. Wasn't sure where user menu fits.

Note

Original attachments:

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 6:50 UTC

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 6:50 UTC

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Aug 23, 2024 at 8:00 UTC (comment 1)

  • Component changed from mc-skin to mc-core

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Aug 23, 2024 at 8:00 UTC (comment 2)

Related to #3442.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Aug 23, 2024 at 10:16 UTC (comment 3)

From my previous experience using this ticket tracker, I never get notifications.

I have fixed them in #4557 - are you still not getting anything?

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Aug 23, 2024 at 10:21 UTC (comment 4)

Few comments:

Compress the current subdirectory (tar.xz using 7z)

Why do you want to offer this alternative? (honest question)

sha512sum

At least on Mac this doesn't exist, can you use a fallback to shasum -a512 ?

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 16:37 UTC (comment 3.5)

Replying to zaytsev:

From my previous experience using this ticket tracker, I never get notifications.

I have fixed them in #4557 - are you still not getting anything?

It's working now. Thank you for fixing it!

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 16:50 UTC (comment 4.6)

Replying to zaytsev:

Few comments:

Compress the current subdirectory (tar.xz using 7z)

Why do you want to offer this alternative? (honest question)

Some systems still use older XZ that is not multi-threaded. But even on systems with newer XZ, 7z is sill faster (especially if you use the "ASMC" compiled version). Also, in my local menu I add "-slp -m0=lzma2 -mx=9 -mfb=256 -md=256m -mlc=4 -ms=on" and get better compression but it was too much for default config ;-)

sha512sum

At least on Mac this doesn't exist, can you use a fallback to shasum -a512 ?

It should be trivial, assuming MacOS has the "which" command.
Is sha256sum also missing?

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 16:59 UTC (comment 7)

Everyone,
If you look closely you will see that I switched to using "7z" instead of '7za', '7zz' etc.
The main reason is that Debian/Ubuntu already switched from p7zip to 7zip and provide cli only using wrappers (7z, 7za, 7zr, p7zip), actual binaries are not even in the path.

Since I can't check all other Distros and OS's, could you please report which variants of '7z' are available on you system.

Thanks.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Aug 23, 2024 at 17:08 UTC (comment 8)

It should be trivial, assuming MacOS has the "which" command. Is sha256sum also missing?

zaytsev@Yurys-MBP % which sha256sum
sha256sum not found

Since I can't check all other Distros and OS's, could you please report which variants of '7z' are available on you system.

On macOS only 7zz is provided via the sevenzip homebrew package:

https://formulae.brew.sh/formula/sevenzip

On BSD I think 7z is provided via the p7zip port.

On Red Hat you have 7za via p7zip package.

You can check other distros yourself using the service from our friend Mykola:

https://pkgs.org/search/?q=7zip

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 18:23 UTC (comment 6.9)

Attaching an updated version of the 2nd patch which minor fix and shasum hashing as a default, since it is a part of Perl and should be available by default on more systems(shaXXXsum is the fallback).

Will start working on 7z support on "all" systems.

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 18:23 UTC

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 23, 2024 at 19:22 UTC (comment 10)

I just reviewed all 7zip related code and '7za' is used in most places with fallback to 7z in some places and 7z/7zz/7zr in other.

This is what 7zip doc say:

Alone         7za.exe: Standalone version of 7-Zip console that supports only 7z/xz/cab/zip/gzip/bzip2/tar.
Alone2        7zz.exe: Standalone version of 7-Zip console that supports all formats.
Alone7z       7zr.exe: Standalone version of 7-Zip console that supports only 7z (reduced version)

Should we unify that with something like:

P7ZIP=`which 7zz 2>/dev/null` || P7ZIP=`which 7z 2>/dev/null` || P7ZIP=`which 7za 2>/dev/null`

Here is my reasoning:
'7zz' should be the first option since it is an ASM optimized pre-built binary with complete format support
'7z' Non-ASM built equivalent of 7zz and is the default on most modern Linux systems
'7za' if available, is the binary that should support all uses on older and systems with p7zip systems
I think we can drop '7zr' since it is only suitable for .7z archives and I believe no distro/os shipping it alone.

P.S.
I am suprised Fedora and Arch do not ship 7zip package...

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 27, 2024 at 3:31 UTC (comment 11)

I am attaching a new version of the main (second) patch.
It still uses the default '7z' (until we decide which variant to use).
I also added an option to use ENV variable for compression options.
Those are the values I set in my .profile if anyone is interested:

export COMPRESS_OPTS_7Z="-slp -myx=9 -mx=9 -mfb=256 -md=256m -mlc=4"
export COMPRESS_OPTS_ZSTD="--long --ultra -22"

P.S.
I am having difficulties understanding what "+' and '=' conditions are supposed to do.
By reading documentation, it looks like '+' is only for "showing" menu items and '=' enables/disables the item. But, it doesn't work as I understand it.
It would be nice if anyone could explain how those should be used.

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 27, 2024 at 3:31 UTC

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 29, 2024 at 19:23 UTC

New version of the patch with 7z binary detection and various fixes

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 30, 2024 at 18:46 UTC

Updated version of the main patch with more compression options, fixes, improvments and shortcuts reorganization

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Aug 30, 2024 at 18:47 UTC

Optional patch to add various GUI tools

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 4, 2024 at 12:30 UTC (comment 12)

Related to #3290.

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Sep 25, 2024 at 2:21 UTC

Updated version of the main patch with various fixes

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 25, 2024 at 7:26 UTC

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 25, 2024 at 7:34 UTC

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 25, 2024 at 7:35 UTC

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 25, 2024 at 7:35 UTC (comment 16)

We had a discussion recently about using command -v instead of which:

https://github.com/search?q=repo%3AMidnightCommander%2Fmc%20%22command%20-v%22&type=code

Otherwise I don't have much to say / strong opinions about menus. I use them very rarely. Are you finished with your work eugenesan?

Andrew, maybe we can take it, apparently it will close a number of open requests like you linked. I can make a branch.

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Sep 25, 2024 at 16:15 UTC

Updated version of the main patch with "which" replacement and shasum preference reversal (non-perl version is 2x faster)

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Sep 25, 2024 at 16:31 UTC (comment 17)

@zaytsev
I've uploaded a new version with "which" replaced.

I believe the patch is ready and tested.
We still have time before next release, in case people will report bugs.

P.S.
I was wondering, can anyone think of a simple way to pass a shell variable from menu "scriptlet" to input dialog (

parameter =
)?
That would allow to replace all the "echo"/"read" interactions for menu commands with proper input dialog.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 25, 2024 at 17:43 UTC (comment 18)

I was wondering, can anyone think of a simple way to pass a shell variable from menu "scriptlet" to input dialog

The dialogs are shown before the script is "compiled" to run in the shell. So you cannot do it unless we make a "zenity" mode for mc and the scripts can call mc to show an overlay dialog :) But I'm not sure this is really necessary. You can just say something like press enter for default (basename)... I don't think the variable values add much value.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Sep 25, 2024 at 17:45 UTC (comment 19)

Those are the values I set in my .profile if anyone is interested

Actually there are official variables XZ_OPT & ZSTD_OPT - is there any particular reason why you use your own?

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Sep 25, 2024 at 19:22 UTC (comment 19.20)

Replying to zaytsev:

Those are the values I set in my .profile if anyone is interested

Actually there are official variables XZ_OPT & ZSTD_OPT - is there any particular reason why you use your own?

Good point.
According to https://github.com/facebook/zstd/blob/dev/programs/README.md ZSTD offers only ZSTD_CLEVEL and I needed to pass other options. 7zip lacks envvar support completely.

I'd name those differently but didn't want interference and variables cannot start with a number so 7Z_OPT wasn't an option.

I needed a way to be able to change default compression options but didn't want to push those in usermenu directly. Those variable are harmless but can be very useful in usermenu context.
Maybe we should rename those to MC_MENU_*_OPT?

Also, I found a last minute bug and will be pushing a new patch momentarily.

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Sep 25, 2024 at 19:23 UTC

Last minute fix

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Sep 25, 2024 at 19:23 UTC

Latest optional patch with addition of GUI tools

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Oct 10, 2024 at 5:31 UTC (comment 21)

  • Milestone changed from Future Releases to 4.8.33

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Oct 22, 2024 at 13:17 UTC (comment 22)

The patch 0001-menu-drop-archaic-items-modernize-archival-options-v9.patch doesn't apply at all. Are you sure you're on latest master?

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Oct 22, 2024 at 16:09 UTC (comment 23)

There are 2 staged patches:

  1. 0001-menu-Reduce-menu-ittems-width-to-allow-longer-text.patch
  2. 0001-menu-drop-archaic-items-modernize-archival-options-v9.patch

The first touches only the visuals and the second one is the actual patch.
They apply cleanly as of today's master.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Oct 23, 2024 at 8:58 UTC (comment 24)

Thanks for the explanation, I was able to apply the patches and test them.

Sorry, my opinion is that whereas this is solid work and maybe a good menu file for you personally, this is not really suitable for stock mc in this form.

Our old files were designed to make menus mostly fit on 80x24 screens and leave a lot of shortcuts for users to customize. Unfortunately, primarily due to the explosion of compression options, the default options list now looks more like an infiniscroll (see attached screenshot).

What I like though is that you tried to keep the shortcuts of the existing options. This is a good thought. Otherwise there will be a huge backlash as the users discover that their muscle memory now causes mc do random things. We need to consider this.

I wonder what we can do to make it fit on one screen for most users again?

  • One easy victim would be Edit a bug report and send it to root
  • View manual page (native) also doesn't make sense - one can press Ctrl-O and man ... - mc option exists specifically because you might wish to use mc viewer
  • Maybe Call the info hypertext browser can also be removed - how often does one do this these days, and what's the advantage as compared to Ctrl+O and info?
  • Maybe Compress the current subdirectory (tar.7z) can be sacrificed, who needs this anyways if 7z has a native container format?
  • Maybe Compress tagged subdirectories (tar.xz using 7z) can be dropped as well, even though you explained why it's useful?
  • Maybe Test compressed archive/image using 7z can be dropped? How different it is from list?
  • Maybe E can get a long extension list to hide it by default?! Is it feasible?

Even with those changes the options I consider "useful" are still more than before, and the new ones are actually not visible :( I'm at a bit of a loss in terms of more "compression" ideas.

Other notes:

  • Copy file to remote host uses rcp - this doesn't make any sense anymore - change to scp?
  • List compressed archive/image using 7z / Test compressed archive/image using 7z only use 7z as fallback, so remove from title and put next to 7z command, if tar -taf is used for tar.*?
  • Same logic applies to all commands that don't necessarily rely on 7z, like Extract compressed tar archive
  • Shortcut for "View Manual Page" changed?
  • t and T should be t and l

I really tried to make it as close to acceptable as possible, but for now I'm exhausted... Just to show what the current state looks like:

Branch: 4571_modernize_usermenu
Initial [98575a5]

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Oct 23, 2024 at 9:17 UTC (comment 25)

Some shortcuts are still changed as compared to original. I wonder if I should take a different strategy if you are not interested in adjusting this to stock requirements - just try to remove myself what I think is not useful, and pick from you what I think is useful...

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Oct 23, 2024 at 17:25 UTC

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Oct 23, 2024 at 17:54 UTC (comment 26)

diff --git a/src/usermenu.c b/src/usermenu.c
index 1ec961153..9bc4db4cd 100644
--- a/src/usermenu.c
+++ b/src/usermenu.c
@@ -1146,8 +1146,8 @@ user_menu_cmd (const Widget *edit_widget, const char *menu_file, int selected_en
             max_cols = MIN (MAX (max_cols, col), MAX_ENTRY_LEN);
 
             /* Create listbox */
-            listbox = listbox_window_new (entries->len, max_cols + 2, _("User menu"),
-                                          "[Edit Menu File]");
+            listbox = listbox_window_new (MIN (entries->len, LINES - 6), max_cols + 2,
+                                          _("User menu"), "[Edit Menu File]");
             /* insert all the items found */
             for (i = 0; i < entries->len; i++)
             {

@mc-butler
Copy link
Author

Changed by eugenesan (eugenesan@….com) on Oct 23, 2024 at 20:04 UTC (comment 27)

Thank you for the feedback.

Following your ideas, I am thinking about:

  1. Where 7z is a separate item, I am going to use 7z if available and fallback to default compressor
  2. I am going to reorganize the items so that the most frequent ones are in the first 18 (number of items visible at 80x24).
  3. Review all the shortcuts/items and make sure they are as close to classic ones as possible.

I think I can remove ~6 items.

Also, we should consider fixing/implementing one of the following:

  1. Combining two menu items by making one of them hidden. For example:
      x Do action (X for verbose)
    

    instead of:

      x Do action
      X Do verbose action
    

    According to the manual this should be possible but I could not make it work. Maybe someone can take a look?

  2. Some kind of "prompt" asking for a secondary choice like list compression option, verbose/non-verbose, test/list etc.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Jan 9, 2025 at 10:58 UTC (comment 28)

  • Milestone changed from 4.8.33 to 4.8.34

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: medium Has the potential to affect progress
Development

No branches or pull requests

1 participant