mc.ext fixes

[zaytsev]

Right now, there are regexes that either are too greedy, like regex/[Mm]akefile$ or simply do not work like regex/^Makefile.(PL|pl)$, because the paths tested are absolute, not relative; my suggested fix is to prepend a slash.

Additionally, I could implement ​https://github.com/MidnightCommander/mc/pull/83 while we are at it.

[andrew_b]

Replying to zaytsev:

Additionally, I could implement ​https://github.com/MidnightCommander/mc/pull/83 while we are at it.

#3115

[zaytsev]

Thank you, Andrew!

[zaytsev]

Branch: 3578_mcext_fixes.
Initial changeset: [cb146b74237aeae7acb96dde94ea3e655c6b84d5].

Please review!

[andrew_b]

Replying to zaytsev:

Thank you, Andrew!

Cannot add comment to #3115 due to circular dependencies (block and block by itself).

[zaytsev]

Cannot add comment to #3115 due to circular dependencies (block and block by itself).

Sorry, it's my fault :-/ should be fixed now.

[andrew_b]

Replying to zaytsev:

Branch: 3578_mcext_fixes.
Initial changeset: [cb146b74237aeae7acb96dde94ea3e655c6b84d5].

Please review!

-regex/[Mm]akefile$
+regex//[Mm]akefile$

Probably

-regex/[Mm]akefile$
+shell/i/Makefile

---

-regex/^Makefile.(PL|pl)$
+regex//Makefile\.(PL|pl)$

Probably

-regex/^Makefile.(PL|pl)$
+shell/i/Makefile.pl

[zaytsev]

shell/i/Makefile

This would be wrong, because this would match *makefile; as far as I understand the idea behind this regex was to match only files with name "Makefile" or "makefile", so that if you press Enter on them it would ask for parameters and run make. Files like "strangemakefile" for example should not be matched.

shell/i/Makefile.pl

Same here, this was originally supposed to match Perl MakeMaker? files, they have the filename "Makefile.pl" or "Makefile.PL", a shell expression like you suggest will match also "MyMakefile?.pL" for example... not sure if this is desired.

[zaytsev]

OK, I hope the mess is sorted out now.

[andrew_b]

Replying to zaytsev:

Right now, there are regexes that either are too greedy, like regex/[Mm]akefile$ or simply do not work like regex/^Makefile.(PL|pl)$, because the paths tested are absolute

I don't quite understand the essence of this bug. Could you please provide an example?

[zaytsev]

Bug 1: regex/[Mm]akefile$

This will match "myMakefile", instead of only "Makefile" or "makefile". Reproducer:

a) touch myMakefile
b) position the cursor on "myMakefile"
c) press Enter.

A menu will appear, but it should not. Fix: add an extra / at the beginning of the expression.

Bug 2: regex/^Makefile.(PL|pl)$

This expression will NEVER be matched. Reproducer:

a) echo 'print "foo\n"' > Makefile.PL
b) position the cursor on "Makefile.PL"
c) press Enter.

Nothing will happen. Expected result: the file should be executed through perl. Fix: replace ^ with /.

[and]

mc.ext shell documentation is unclear

shell/.myfile
will match all filenames with .myfile suffix (special handling code about '.')

shell/myfile
will only match when fullpathname is 'myfile' (will never be true)

shell//tmp/myfile
will work for /tmp/myfile but this is not what we want

[zaytsev]

Replying to and:

new suffix word should be introduces for easy right hand matching rules

I believe that my solution (regex//myfile$) doesn't have any serious disadvantages, so a new suffix is not really necessary. What do you think, am I missing something?

[and]

Your solution is ok for the two cases (I'm unsure about ls-lR stuff)
but there are more cases that will not work as wanted
all shell/ rules without '.' definition are suspicious

$ grep -P '^shell/(i/|)?+[^.]' mc.ext 
shell/read.me
shell/Imakefile

[zaytsev]

Hi Andreas, you are right, thanks! I've pushed another commit, what do you think?

[andrew_b]

Replying to zaytsev:

because the paths tested are absolute, not relative;

I think that is the bug. mc.ext should get file name only, not full path.

my suggested fix is to prepend a slash.

User can be confused with that.

Bug with full name in mc.ext is fixed in the last commit of the branch.

[zaytsev]

Andrew, thank you for your help. Cleaned up everything, rebased against master. OK?

New initial changeset: e7bdd9.

[zaytsev]

Merged to master: [f74754c03de399b372e884c691ad1ea4b156e82c].

[andrew_b]

Don't forget to remove merged branches.

[zaytsev]

Ok, sorry.