[brlcad-tracker] [ brlcad-Patches-1377410 ] Bigger, Better vi command line editing

[SourceForge.net <no...@so...>]

Patches item #1377410, was opened at 2005-12-09 22:14
Message generated for change (Comment added) made by swcto
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=640804&aid=1377410&group_id=105292

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Geometry Editing
Group: None
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: Someone Who Cares (swcto)
Assigned to: Sean Morrison (brlcad)
Summary: Bigger, Better vi command line editing

Initial Comment:
Brought vi command line editing up to the same level as
commonly found in bash/ksh/etc.

Addition of many edit-mode commands

Addition of / and ? searches of command history.

files changed:

src/tclscripts/mged/openw.tcl:
initialisation of new vi_state stucture members
slight change to command window text object
instantiation to allow use of text object's undo
functionality

src/tclscripts/mged/text.tcl:
Bulk of changes and additions.
Some changes to key bindings, and the addition of one
more input "mode"

src/mged/history.c:
Changes to hist_prev()/hist_next() to allow pattern
matching.
Addition of "hist cur" subcommand (returns current
history element)

Note1: There exists another file,
src/tclscripts/lib/Command.tcl, which also implements
much of what text.tcl implements, but as a class.  It
seems unused, and I left it alone.  If that class is or
should be used, I would happily consider updating it.

Note2: There is an implicit space at the end of lines
in the command window; this causes some slight
functionality differences.  I was inclined to remove
it, for consistency reasons, but thought I should seek
input before doing so.  It appears to be only a
programming convinience to do with the highlight
cursor. read: someone was lazy.

Note3: compiled/tested against 7.6.4.

Note4: tested rather thoroughly, but not completely
exhaustively.  If bugs are encountered, I will gladly
assist in their rectification.  You may add "swcto" to
the SourceForge project to facilitate this if you wish.

----------------------------------------------------------------------

>Comment By: Someone Who Cares (swcto)
Date: 2006-06-15 01:55

Message:
Logged In: YES 
user_id=513595

Re: moved key bindings.
As I recall, Escape used to be aborting input mode
correctly, but by happenstance.  I had to add another edit
mode for, hmmm... the mists of time..., for *something*! 
Some command submode that needed different bindings for some
reason.  Anyways, when the input modes became a bit more
complex, the stateful changes to bindings had to become a
bit more explicit.

Re: duplicate CLE mode on Archer.
Perhaps when it becomes more stable I'll propogate the vi
command editing, hopefully in some fashion other than total
duplication...

Re: Note 2.
I may just explore removing the implicit space, and or
libedit if some spare time comes up.

It was my pleasure doing this work; the last vi work I did
(I'm a systems programmer) was fixing hairy multi-byte bugs
in a multi-byted build for the Japanese version of Iterix...
 It was a nice change of vernacular, going from C to Tcl. 
And I've always liked Tcl anyways.

Ciao,

james

----------------------------------------------------------------------

Comment By: Sean Morrison (brlcad)
Date: 2006-05-17 17:51

Message:
Logged In: YES 
user_id=785737

Thanks, the patch looks great!  Big apologies on the response delay, it was a 
large patch to review and the Windows release has created quite a backlog.  
The only problem I noticed was a simple bug introduced on the hist_prev/
hist_next where the pattern would be non-null but unset due to undefined 
behaviors causing a crash.  Trivial to fix, though.  Other than that, it looks 
good to go.

The only curiosity/question I see is the removal/move of the <Return> and 
<Escape> bindings in text.tcl from set_text_key_bindings, which if I follow 
the patch correctly were basically fixed to behave more faithfully to vi (e.g. 
Escape was incorrectly not aborting in vi edit mode) and moved to a default 
start-up binding.

Note1: the Command.tcl class is used by the follow on 'Archer' geometry 
editing/modeler interface (which hasn't become stable or been released yet 
for *nix platforms) which uses more of an object oriented design approach 
(hence the classes) and a completely different gui.  So in short, it is being 
used just not quite so widely as mged just yet as it's under active 
development.  We will be announcing that though as soon as all the cross-
platform compilation issues get worked out.

Note2: You are right on the money.  Removing it would probably be a good 
thing if it still written to behave basically the same as one would expect 
consistently.  Another idea worth considering is moving towards something 
like the editline library (libedit, a BSD licensed version of GNU Readline) 
though that is of course a much bigger change.

Note3: Applied just fine to the latest sources (7.8.1)

Note4: I haven't encountered any other bugs yet, but the interest in getting 
involved in the project is highly appreciated and desired.  Feel free to stop by 
the #brlcad irc channel on the freenode network (irc.freenode.net:6667) and/
or check out the HACKING file - which is basically the BRL-CAD Developer's 
Guide.  As new devs are getting involved, I'd personally like to see more 
discussion and interaction focused on IRC as it's a lot more efficient and 
allows us to collaborate better from my experiences than the alternatives.  
Certainly not required, but definitely useful.

Modal bindings for our modal editor is something I'm sure will be highly 
appreciated by many.. :-)  Thanks again for your hard work; the patch will 
likely be distributed in 7.8.4 unless another bug or some other issue comes 
up.

Cheers!
Sean



----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=640804&aid=1377410&group_id=105292