Re: [Midnight Commander] #55: savannah: tab completion vs. spaces and escaping
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450
Branch state: no branch | Votes for changeset:
---------------------------+----------------------------------
Changes (by slavazanko):
* owner: => slavazanko
* status: new => accepted
* branch_state: => no branch
--
Ticket URL: <www.midnight-commander.org/ticket/55#comment:7>
Midnight Commander <http://www.midnight-commander.org>
Midnight Development Center
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450
---------------------------+----------------------------------
Changes (by slavazanko):
* cc: egmont (added)
* branch_state: no branch => on review
Comment:
Created branch 55_filename_complete (parent: master)
Initial changeset:b701c1e353ea66e1a4007097721d46b641c5386e
Review, please.
--
Ticket URL: <www.midnight-commander.org/ticket/55#comment:8>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450
---------------------------+----------------------------------
Changes (by andrew_b):
* branch_state: on review => on rework
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:9>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Resolution: | Keywords:
Branch state: on review | Votes for changeset:
---------------------------+----------------------------------
Changes (by slavazanko):
* branch_state: on rework => on review
* milestone: 4.8 => 4.8.3
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:11>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.3
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
Branch state: on review | Votes for changeset:
---------------------------+----------------------------------
Comment (by ossi):
in lib/widget/input_complete.c, the tab char should also be treated like
that. you can insert a tab with ctrl-q tab easily, even if this is not
really useful in the normal case.
INPUT_COMPLETE_FILES_ESC sounds like a misnomer to me. at the level of the
input api it doesn't relate to files at all (or at least it shouldn't - i
can't see from the patch alone whether that's actually the case).
INPUT_COMPLETE_SPACE_ESC sounds like a better name.
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:12>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.3
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
Branch state: on review | Votes for changeset:
---------------------------+----------------------------------
Comment (by slavazanko):
Ossi, your wishes have been realized in temporary changesets
[changeset:dac8b78fb53c76ce6d018f41cc679c97c83aef28 dac8b78fb] and
[changeset:b530609cb335d13c04bd9169cf2a21d06c29f7d2 b530609cb]. Thanks.
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:13>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.3
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
Branch state: on review | Votes for changeset:
---------------------------+----------------------------------
Comment (by ossi):
actually, the logic in lib/widget/input_complete.c is broken. i think it
should be more or less like this:
{{{
// no space, no tab here.
// XXX this list seems pretty arbitrary to me. if shell, missing at least:
()&
if (strchr (";|<>", *s) != NULL)
break;
if ((*s == 32 || *s == 9) &&
(!(in->completion_flags & INPUT_COMPLETE_SPACE_ESC) ||
!strutils_is_char_escaped (in->buffer, s)))
break;
}}}
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:14>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.3
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
---------------------------+----------------------------------
Changes (by slavazanko):
* branch_state: on review => on rework
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:15>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
---------------------------+----------------------------------
Changes (by slavazanko):
* branch_state: on rework => on review
* milestone: 4.8.3 => 4.8.4
Comment:
rebased to latest master and have respected ossi's thoughts.
Review, please.
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:16>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.4
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
Branch state: on review | Votes for changeset:
---------------------------+----------------------------------
Comment (by ossi):
a recommendation for the commit message: use the past tense for the broken
things that your commit fixes. it makes no sense to describe *some* state
in the present tense, because it is not clear whether that's the new or
the old state.
your re-definition of INPUT_COMPLETE_DEFAULT makes it a misnomer - it's
more like INPUT_COMPLETE_NONE now.
anyway, the more i look into this code, the less i understand. what
exactly is the above fragment supposed to do? the "stop chars" in the
strchr are applied irrespective of mode, which makes no sense to me at
all. what is INPUT_COMPLETE_SPACE_ESC meant to actually do? as far as i
can see, there are only two modes: plain (regular line inputs) and shell-
like (command prompt. fwiw, the built-in cd should just pretend to be a
regular cd command (i.e., follow the normal shell splitting and quoting
rules), then there would be no ambiguity). i can't find the code which is
supposed to quote completions for the command prompt - am i or the code
missing something?
try_complete() contains a lot of fuzzy logic, in particular with backslash
escaping being handled rather arbitrarily or not at all.
strutils_is_char_escaped() should be consistently applied, with regard to
the mode.
INPUT_COMPLETE_DIRNAMES needs to be split out from INPUT_COMPLETE_CD,
otherwise the "cd" detection magic is activated outside command prompt
context as well.
generally, i'd suggest that you *precisely* describe the intended
semantics of the INPUT_COMPLETE_* enum values next to their declarations,
and then match these descriptions against both the implementation and the
usages.
wow. two hours gone.
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:17>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.4
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
---------------------------+----------------------------------
Changes (by slavazanko):
* branch_state: on review => on rework
Comment:
> a recommendation for the commit message: use the past tense for the
broken things that your commit fixes. it makes no sense to describe *some*
state in the present tense, because it is not clear whether that's the new
or the old state.
Thank for explanation, my English isn't good, and I accept all lessons and
criticism.
> your re-definition of INPUT_COMPLETE_DEFAULT makes it a misnomer - it's
more like INPUT_COMPLETE_NONE now.
Yep, you right, INPUT_COMPLETE_NONE have more sense now. I changed the
name of constant.
All your other notes I have to take a closer look in the evening with a
relaxing cup of tea. So, ticket going to 'rework' stage.
> wow. two hours gone.
But I hope, you got a fun in this two hours, because a fun it an engine of
!OpenSource (in most cases) :)
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:18>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
Branch state: on rework | Votes for changeset:
---------------------------+----------------------------------
* milestone: 4.8.4 => 4.8.5
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:19>
Ticket System
Type: defect | Status: accepted
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
Branch state: on rework | Votes for changeset:
Changes (by andrew_b):
* milestone: 4.8.5 => Future Releases
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:20>
Ticket System
---------------------------+----------------------------------
Type: defect | Status: accepted
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
---------------------------+----------------------------------
Changes (by slavazanko):
* branch_state: on rework => on review
* milestone: Future Releases => 4.8.8
Comment:
Branch was recreated. See
[changeset:5b3bddeb2c1fe52180c36d0a133d77508a26af9c 55_filename_complete]
for details. Review & vote, please.
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:21>
Ticket System
---------------------------+----------------------------------
Reporter: slavazanko | Owner: slavazanko
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.8
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
---------------------------+----------------------------------
Changes (by andrew_b):
* votes: => andrew_b
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:22>
Ticket System
Type: defect | Status: accepted
Priority: major | Milestone: 4.8.8
Component: mc-core | Version: master
Resolution: | Keywords:
Blocked By: | Blocking: 2450, 2626
---------------------------+-----------------------------------------
Changes (by angel_il):
* votes: andrew_b => andrew_b angel_il
* branch_state: on review => approved
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:23>
Ticket System
---------------------------+----------------------------------------
Type: defect | Status: testing
Component: mc-core | Version: master
Blocked By: | Blocking:
Branch state: merged | Votes for changeset: committed-master
---------------------------+----------------------------------------
Changes (by slavazanko):
* status: accepted => testing
* votes: andrew_b angel_il => committed-master
* resolution: => fixed
* blocking: 2450, 2626 =>
* branch_state: approved => merged
Comment:
merged to master:
{{{
git log --pretty=oneline 74d71e7..0608af2
}}}
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:24>
Ticket System
---------------------------+----------------------------------------
Reporter: slavazanko | Owner: slavazanko
Component: mc-core | Version: master
Resolution: fixed | Keywords:
Blocked By: | Blocking:
Branch state: merged | Votes for changeset: committed-master
---------------------------+----------------------------------------
Changes (by slavazanko):
* status: testing => closed
--
Ticket URL: <http://www.midnight-commander.org/ticket/55#comment:25>