FLTK logo

STR #3305

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]

STR #3305

Application:FLTK Library
Status:1 - Closed w/Resolution
Priority:3 - Moderate, e.g. unable to compile the software
Scope:3 - Applies to all machines and operating systems
Subsystem:Core Library
Summary:Fl_Text_Editor: add a method to support tab navigation
Version:1.3-current
Created By:greg.ercolano
Assigned To:greg.ercolano
Fix Version:1.3.4 (SVN: v11808)
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size  
 
#1 ianmacarthur
01:58 May 18, 2016
editor-tab-navigation-1.cxx
1k
 
 
#2 ianmacarthur
09:03 May 18, 2016
editor-tab-navigation-2.cxx
2k
 
 
#3 greg.ercolano
10:24 May 18, 2016
Fl_Text_Editor-tab_nav-v1.patch
2k
 
 
#4 ianmacarthur
02:15 May 19, 2016
editor-tab-navigation-3.cxx
4k
 
 
#5 greg.ercolano
09:23 May 19, 2016
Fl_Text_Editor-tab_nav-v3.patch
4k
 
 
#6 AlbrechtS
09:53 May 25, 2016
Fl_Text_Editor-tab_nav-v3a.patch
4k
 
 
#7 greg.ercolano
12:02 Jul 14, 2016
keynav-test.cxx
1k
 
     

Trouble Report Comments:


Name/Time/Date Text  
 
#1 greg.ercolano
12:36 May 17, 2016
Users trying to make a database form will find it hard to support
tab navigation if they use an Fl_Text_Editor for multiline input
for one of the fields, as tab is used as an input character.

1) There should be a method, similar to Fl_Input's tab_nav(0|1) to make
it easy to use in this context.

2) Perhaps too, a new wrapper widget, e.g. Fl_Text_Input, that implements
Fl_Text_Editor with this flag set in the ctor, so form users can simply
use it instead.

3) Additionally, such a widget could maybe implement an Fl_Text_Buffer internally
so that one doesn't have to make the separate buffer instance and buffer()
assignment, making it "easy" to use, esp. in Fluid. (As it is, a new user
would find it challenging to make an Fl_Text_Editor/Display in fluid, having
to know in advance about not only the details of Fl_Text_Buffer, but how to
do these multiple steps all within fluid)

#1 for sure, #2 maybe, #3 for debate.
 
 
#2 ianmacarthur
01:51 May 18, 2016
Point #1:

Note that I would contend that we already have #1 (well, sort of) though the API to access it is somewhat obscure.

The crux to this is that you can tell the Fl_Text_Editor (and its sibling Fl_Text_Display) specifically which keys to process, and what to do with them.

By default, most keys are set to the "default" key handler, and so are processed by the widget.

However, it is possible to set *any* key to use the "ignore" handler instead - this is what is done by default in the editor the widget for the ESC key, to ensure that fltk's usual ESC key handling happens and is not subverted by the editor widget. (And you can also define custom handlers too, should you be so inclined.)

It is fairly straightforward therefore (albeit cryptic) to redirect FL_Tab to the "ignore" handler, and the editor widget will then allow tab navigation "normally" since it will no longer grab the TAB key for itself.

For an editor widget called "edit_1" this would look like:

  // Tell the editor to ignore tabs
  edit_1->add_key_binding (FL_Tab, FL_TEXT_EDITOR_ANY_STATE, Fl_Text_Editor::kf_ignore);

With that line implemented (probably where the widget is instantiated) you end up with an editor widget that will participate cleanly in TAB navigation but which loses the ability to type any TAB characters.

For the "forms entry" case outlined in Greg's note, this is probably sufficient.

A more complex case might alter the STATE for the key handler from FL_TEXT_EDITOR_ANY_STATE so that it handled some or all of TAB, ALT+TAB, SHIFT+TAB, CTRL+TAB differently, opening the prospect of the user being able to use a "bare" TAB for navigation, and (say) a SHIFT+TAB to enter a TAB character into the form.


Point #2:

Seems reasonable, as noted above it ought to be a fairly simple extension to the stock editor widget, just done in the constructor?

Point #3:

If #2, then I guess #3 is painless. However, for more complex editor uses there are advantages to managing the text buffer and the editor view independently.
 
 
#3 ianmacarthur
01:59 May 18, 2016
Posted example file:

editor-tab-navigation-1.cxx

illustrating an editor widget participating in TAB navigation "normally".
 
 
#4 ianmacarthur
09:05 May 18, 2016
Posted revised editor code that shows how to allow regular TAB navigation, but also allow the user to type CTRL+TAB into the editor widget, if they actually need to type a TAB, rather than just grabbing all TAB keystrokes as navigation keys.

Not the most intuitive way to type a TAB mind you, but I was just trying to illustrate the point.

File: editor-tab-navigation-2.cxx

Tested on WIN7 only; I assume it would work "anywhere" though.
 
 
#5 greg.ercolano
09:38 May 18, 2016
In Fl_Input, the docs say ^I can be used to insert a tab.
We should probably support the same for consistency.. opinions?

I'm about to suggest a patch that uses your technique.
Perhaps that patch should include having ^I default behavior to
insert a tab?

Currently ^I isn't bound in Fl_Text_Editor, so it does nothing when typed.

However, our test/editor demo seems to overload it for inserting a file.
Perhaps the editor demo should use Alt-I instead.

An aside:
    While writing the above patch, I realized a separate issue
    outlined in STR#3306.
 
 
#6 greg.ercolano
10:24 May 18, 2016
Adding Fl_Text_Editor-tab_nav-v1.patch which simply adds
a tab_nav() set/get method. Defaults remain the same.

As implemented, this is actually ABI safe.

Advising tab_nav(void) be left non-const until STR 3306 is resolved,
then ABI guards can be added so it can all be fixed properly in 1.4
 
 
#7 ianmacarthur
02:14 May 19, 2016
Updated example code to also support "CTRL+i" for TAB insertion, as suggested by Greg based on what we document for the Fl_Input widget family.

File:  editor-tab-navigation-3.cxx
 
 
#8 ianmacarthur
02:30 May 19, 2016
@Greg

I was looking at the Fl_Text_Editor-tab_nav-v1.patch file, and I think the comment here is awry:

+    \returns 1 if Tab advances focus (default), 0 if Tab inserts tab characters.

I think the default is the "Tab inserts tab characters" case, is it not?


Anyway, that got me wondering whether testing that bound_key_function() with FL_TEXT_EDITOR_ANY_STATE is bound to "kf_ignore", is really a robust way to test for this?

I'm thinking that a user might have encoded some set of TAB key combos, one of which matches "kf_ignore", and in that circumstance we would report that tab_nav() was asserted, when in fact it might not be?

Though I have not tested, so maybe this is OK as is?

I guess we would perhaps want to add a member variable to Fl_Text_Editor to explicitly record what the user had passed to tab_nav() and report that, giving us a "direct" way of assessing whether the user had requested tab_nav or not.

But I imagine that would be more of an issue at the ABI level than what you are proposing here.
 
 
#9 greg.ercolano
08:33 May 19, 2016
> I think the default is the "Tab inserts tab characters" case,
> is it not?

    Yes, thanks.

    That was a copy/paste carryover from Fl_Input's tab_nav() (where it IS the default)
    which I forgot to modify.

> Anyway, that got me wondering whether testing that bound_key_function()
> with FL_TEXT_EDITOR_ANY_STATE is bound to "kf_ignore", is really a
> robust way to test for this?
>
> I'm thinking that a user might have encoded some set of TAB key combos,
> one of which matches "kf_ignore", and in that circumstance we would
> report that tab_nav() was asserted, when in fact it might not be?

    Ah, do you mean changing the use of FL_TEXT_EDITOR_ANY_STATE to a
    different value, e.g. 0?

    Could be -- we surely only want to affect Tab without modifiers.
    (Shift-Tab /already/ seems to be ignored, which is expected for
    Tab nav'ing in reverse, so I think we need to only focus on fwd-nav)

    Or are you referring to the use of kf_ignore to test in a more
    general way, even with the above mods to only focus on Tab with no modifiers?
    I figure if a user messes around with the bindings for Tab,
    they shouldn't be surprised if the value of tab_nav() is affected.
 
 
#10 greg.ercolano
08:36 May 19, 2016
Oh, and a separate issue: the FL_TEXT_EDITOR_ANY_STATE value
could use some docs; it appears to currently be undocumented.

I was confused about what it does, and see my use of it in tab_nav()
is probably wrong, the value to use should probably be 0.
And I may need to make a separate kf_ignore binding for Shift-Tab.
Will do some tests.
 
 
#11 greg.ercolano
08:57 May 19, 2016
BTW, regarding ^I to insert tabs, I was thinking we should make
that /default/ behavior for Fl_Text_Editor, to match Fl_Input + friend's
default behavior.

In other words, an additional entry in the default_key_bindings[]
definition in Fl_Text_Editor.cxx, something to the effect of:

  ..
  { FL_Tab, FL_CTRL, Fl_Text_Editor::kf_tab },
  ..    

..where "kf_tab" would be a new function that simply inserts
a tab character, with the same or similar code to what your
ctrl_i_to_tab() function does in your editor-tab-navigation-3.cxx file.

Comments?
 
 
#12 greg.ercolano
09:23 May 19, 2016
Attaching v3 patch, which addresses these issues:

    o Only binds/unbinds the *un-modified* Tab key
    o Docs fixed as per Ian's note about defaults
    o Docs simplified/clarified
    o Made the tab_nav(void) method const.
      This has the side effect of forcing this to be an ABI 1.3.4
      feature, because the existing bound_key_function()
      method tab_nav() uses is itself not const. (The ABI restriction
      can be removed if we release tab_nav(void) as a non-const method)
    o Due to the ABI feature, added \version doxygen tags indicating
      this is a 1.3.4 feature only.

Not addressed: ^I default behavior inserting a tab. Want to get
comments on that first, and would address that as a separate patch.
 
 
#13 AlbrechtS
09:53 May 25, 2016
Notes concerning Fl_Text_Editor-tab_nav-v3.patch:

(1) I tried to fix a few formatting issues (Doxygen docs) and a typo. The modified patch Fl_Text_Editor-tab_nav-v3a.patch is attached.

(2) I believe that these new methods are not (closely, really) related to Fl::OPTION_ARROW_FOCUS, so the two instances of
"\see ... Fl::OPTION_ARROW_FOCUS." should probably be changed (remove Fl::OPTION_ARROW_FOCUS). What do you think?

(3) If we intend to change the implementation, then the new methods should better not be inlined (in the header file) but be moved to the .cxx file instead. Again, asking for opinions.

(4) Question: should we really add this to FLTK 1.3.4 (as the ABI guards in the proposed patch indicate), or should we better do it in the porting branch, i.e. 1.4.0? My opinion is that we should also address the ^I insertion and I'd prefer an implementation that does not interfere with key definitions (the mentioned implementation change?). If so, this would definitely have to go into 1.4.0, since that change would have potential to break something. But, as above, asking for opinions...


PS: The tab_nav() docs didn't show up in Doxygen because they were guarded by ABI guards which Doxygen didn't know about. I committed svn r11754 that fixes this (by defining FLTK_ABI_VERSION=99999 in Doxygen).
 
 
#14 greg.ercolano
15:32 Jul 13, 2016
@Albrecht:

   #1 -- great.

   #2 -- Yes, I'd agree, no reference to OPTION_ARROW_FOCUS needed;
         in this case they're unrelated issues.

   #3 -- Yes, in general it seems best to keep code in the .cxx files
         unless the implementation is /really/ simple. My goal here was probably
         to just minimize the code deltas.

   #4 -- This is an interesting issue.

         I suppose we want ABI features to be documented regardless of what the
         ABI value was set to when the doxygen docs are built. (This way e.g. the website
         docs can reflect new features as well as old behavior)

         Pretty sure in the past I made separate documentation for old vs. new ABI features,
         so the docs were only consistent with what was built.

         But I suppose it might be best if the docs always reflected old and new behaviors,
         and making sure those docs don't change with the ABI version macro.

         This means the docs need to clearly indicate old and new behavior,
         referring to specific ABI version numbers.

         We might need to make a pass at that before releasing 1.3.4 and regenerating
         the website's docs.
 
 
#15 greg.ercolano
10:33 Jul 14, 2016
Related: I committed a fix to related patch STR #3306 which makes the
bound_key_function() function const when the ABI is set >= 10304.

This addresses the problem mentioned in comment #6 below,
so all patches here that try to deal with that issue are no longer needed.
 
 
#16 greg.ercolano
12:02 Jul 14, 2016
Fixed in Subversion repository.

Assigned to myself, committed as 11808, closed.

Notes on the commit:

    > I used Albrecht's v3a patch as a model

    > Took his suggestion re: moving the implementation into the .cxx
      (so that future changes to the code wouldn't break ABI
      by being inline)

The only reason this mod needed ABI guards at all is because of the
const issue with the bound_key_function(), solved by related STR #3306.
(Didn't want to add this new tab_nav() method as non-const)

Also attaching the test program I used to verify this new tab nav flag.
I'd add this test to the test/unittests program, but it's such a PITA
to add new modules to the Makefile/IDEs, I didn't want to do it. But
that should be done in the future, so tab navigation can be tested for
all widgets.

This commit should be easy to revert if for some reason it's decided
it doesn't make sense for 1.3.4. Seems to me it's a good idea to get
features like this in early, as our release schedule lately has been
few and far between..
 
 
#17 greg.ercolano
13:19 Jul 19, 2016
Also applied to the 1.3-porting branch as r11820.  
     

Return to Bugs & Features ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2024 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.