#588 Smart indent and ctrl+click enhancements

Completed
closed
None
3
2009-07-03
2009-05-24
maXmo
No

1. Smart indent.
Declared as

enu WrapSmartIndentMode=SC_WRAPINDENT_
val SC_WRAPINDENT_FIXED=0
val SC_WRAPINDENT_SAME=1
val SC_WRAPINDENT_INDENT=2

## SCI_SETWRAPINDENTMODE
# Sets how wrapped sublines are placed. Default is fixed.
set void SetWrapIndentMode=2472(int mode,)

## SCI_GETWRAPINDENTMODE
# Retrieve how wrapped sublines are placed. Default is fixed.
get int GetWrapIndentMode=2473(,)

SC_WRAPINDENT_FIXED - current behavior, default
SC_WRAPINDENT_SAME - wrapped lines are aligned to first line indent
SC_WRAPINDENT_INDENT - first line's indent + yet one more indent (as defined by document).

demo: http://i42.tinypic.com/ka5qp1.png (for SC_WRAPINDENT_INDENT)

2. Ctrl+click selects word just like doubleclick. Ctrl+click just much more useful and users miss it from different editors. Now you can do some lame refactoring: while holding Ctrl, click, C, click, V, click, V and so on.

3. Put back SCE_D_WORD3, I have plan for it :)

Discussion

1 2 > >> (Page 1 of 2)
  • maXmo

    maXmo - 2009-05-24

    based on version 1.78

     
  • maXmo

    maXmo - 2009-05-24

    another demo with interesting usecases: http://i40.tinypic.com/iqv2uo.png

     
  • Neil Hodgson

    Neil Hodgson - 2009-05-25

    Use separate items for each change. Otherwise it gets messy when some of this is accepted and some of it isn't.

    For the indent wrapping you should update to the current CVS state since there have been changes that affect some of this code. Notably PositionFromLocation and PositionFromLocationClose have been replaced with PositionFromLocation with a canReturnInvalid argument that can choose 'close' or 'must hit' behaviour.

    There are also several changes that appear to just be stylistic such as using preprocessor definitions rather than enumerations and reordered code. This makes it harder to review.

    Ctrl+Click will eventually be used for discontinuous selection as it is in other applications such as FireFox and Open Office Writer.

     
  • Neil Hodgson

    Neil Hodgson - 2009-05-25
    • priority: 5 --> 3
    • assigned_to: nobody --> nyamatongwe
     
  • Neil Hodgson

    Neil Hodgson - 2009-05-25

    This is more likely to lead to text not being displayed when it is already near the right edge and is then wrapped in a very narrow column. Not sure there is anything better to do although there could be some sort of backing off to say that the wrap indentation position must be some number of pixels from the right border unless that would move it before the left border.

    A minor issue is that the wrap symbol moves the wrap position so that there is an indent even in SC_WRAPINDENT_SAME. May be possible to use the width of the symbol as a minimum rather than something added.

     
  • maXmo

    maXmo - 2009-05-25

    >Ctrl+Click will eventually be used for discontinuous selection as it is in
    >other applications such as FireFox and Open Office Writer.
    Tried the feature in FireFox... Seems like it selects a set of block-level elements and all text insinde them. Can't grasp its purpose... And people ask for Ctrl+Click support in my variant. I find it useful too.

    >This is more likely to lead to text not being displayed when it is already
    >near the right edge and is then wrapped in a very narrow column.
    Yes, I'll improve the wrapping code. Minimum width of 20 seems to be too small, I'll change it to 100.

    >A minor issue is that the wrap symbol moves the wrap position so that
    >there is an indent even in SC_WRAPINDENT_SAME.
    This is intended behavior. Wrap symbol is visible and it seems like part of text so it's not good for it to appear in the indentation space which should be clear.
    ...Oh... it's for symbol near text... Hmm... I see.
    So you propose to use minimum...
    How about this scenario: for symbol near text add symbol width to wrapAddIndent in RefreshStyleData, in LayoutLine if ll->wrapIndent==0, fix it.

    I also think about removing SC_WRAPINDENT_SAME and use wrapVisualStartIndent for a number of additional indents (not spaces) in SC_WRAPINDENT_INDENT mode. Do you think it's better? wrapVisualStartIndent will have different meanings depending on warpIndentMode.

    I also think about removing SC_WRAPINDENT_SAME and leave only SC_WRAPINDENT_INDENT for fixed one extra indent. SC_WRAPINDENT_SAME is used in EditPlus and used to be quite good and useful, but I think SC_WRAPINDENT_INDENT is much better and superior to SC_WRAPINDENT_SAME so it's impractical to include implementation for SC_WRAPINDENT_SAME. I find SC_WRAPINDENT_SAME from EditPlus a nice feature but I'll be happy with SC_WRAPINDENT_INDENT alone. What do you think? Should we ask people about this?
    A special case here is html document as it often gets long chunks of text and many attributes for elements, but I think SC_WRAPINDENT_INDENT is better here too.

     
  • maXmo

    maXmo - 2009-05-25

    >Use separate items for each change.
    I plan to improve logic of wrapping code, but it's intertwined with indentation support code, so it will be more natural to merge it with indentation code changes or should this be postponed to commit of new indentation code?

     
  • Neil Hodgson

    Neil Hodgson - 2009-05-25

    Discontinuous selection is where you make one selection, then add another selection by holding down Ctrl while dragging and so on. One of the common uses is to collect a bunch of things together (such as all the identifiers in a function or similar) then copy all of that text somewhere else.

    The 20 pixels is to prevent it behaving strangely when the window is just a few pixels wide. If it is made much wider then some reasonable uses of thin windows start hiding text.

    > This is intended behavior. Wrap symbol is visible and
    > it seems like part of text so it's not good for it to appear
    > in the indentation space which should be clear.

    But that makes the wrapped lines not line up with the first line which I thought was the whole point of this modification.

    Using the WrapStartIndent as the amount for SC_WRAPINDENT_INDENT sounds more flexible since you can set it to 0 to achieve SC_WRAPINDENT_SAME.

     
  • Neil Hodgson

    Neil Hodgson - 2009-05-25

    Please don't 'improve logic'. Just make the simplest change possible that achieves a specific improvement.

     
  • maXmo

    maXmo - 2009-05-26

    >But that makes the wrapped lines not line up with the first line which I
    >thought was the whole point of this modification.
    Wrap symbols will be aligned to the first line.

     
  • maXmo

    maXmo - 2009-05-27

    This is implementation for smart indent. I precisely followed all code duplication strategies.

     
  • maXmo

    maXmo - 2009-05-28

    May be these 20 pixels should be tied to aveCharWidth? For Courier New 10pt 20 pixels is too small.

     
  • Neil Hodgson

    Neil Hodgson - 2009-05-28

    With SC_WRAPINDENT_FIXED, WrapStartIndent is in character widths but for SC_WRAPINDENT_INDENT it is interpreted as indent widths so often 4 to 8 times larger. The wrapAddIndent calculation can go in a method.

    The 20 pixels in ll->wrapIndent = width - 20 can be made wider and tied to character width since this is about continuing to show some text but the minimum width = 20 should be left as is.

     
  • maXmo

    maXmo - 2009-05-31

    Moved wrapAddIndent calculation to RefreshStyleData. Fixed minimum wrap width.

     
  • Neil Hodgson

    Neil Hodgson - 2009-06-01

    To avoid compiler warnings there should be a cast like so:

    if ((wrapVisualFlags & SC_WRAPVISUALFLAG_START) && (ll->wrapIndent < static_cast<int>(vstyle.aveCharWidth)))

    ll->wrapIndent should not ever be negative.

     
  • maXmo

    maXmo - 2009-06-02

    fixed

     
  • Neil Hodgson

    Neil Hodgson - 2009-06-02

    Still hasn't dealt with the difference in interpretation of the indent amount in SC_WRAPINDENT_FIXED versus SC_WRAPINDENT_INDENT.

     
  • maXmo

    maXmo - 2009-06-03

    reverted to the original scheme

     
  • maXmo

    maXmo - 2009-06-03

    ok, reverted to the original scheme.

     
  • Neil Hodgson

    Neil Hodgson - 2009-06-03

    I was actually after a reason why SC_WRAPINDENT_INDENT should use indent widths rather than character widths. It makes the feature harder to document and understand.

     
  • maXmo

    maXmo - 2009-06-04

    Only three modes are important in fact: _FIXED, _SAME and _INDENT (as described above). There was a proposal to merge _SAME and _INDENT modes by making use of wrapStartIndent. Use of indent widths is necessary to match original _INDENT behavior. As you don't like the merge, it's natural to return to the original scheme, isn't it?

     
  • maXmo

    maXmo - 2009-06-04

    You can't make indent width of aveCharWidth because indent is measured in spaceWidths.
    And current use of aveCharWidth contradicts to the docs BTW.

     
  • Nobody/Anonymous

    Fixed documentation of SCI_SETWRAPSTARTINDENT to match behaviour.

    It is not natural to return to the original modification since this means that SCI_SETWRAPSTARTINDENT means two different things.

     
  • maXmo

    maXmo - 2009-06-04

    In original scheme wrapStartIndent has only one meaning (for _FIXED mode), it's ignored in _SAME and _INDENT modes, see RefreshStyleData for implementation.

     
  • Neil Hodgson

    Neil Hodgson - 2009-06-05

    When checking for minimum width, if there isn't room for 15 characters, it now moves to the base wrapAddIndent position which moves the wrapped text way to the left. It makes a big change when you are making the window thinner and hit the 15 character position.

    if (ll->wrapIndent > width - vstyle.aveCharWidth * 15)
    ll->wrapIndent = wrapAddIndent;

    I thought earlier versions just moved the text to width - vstyle.aveCharWidth * 15 so that it was still indented quite a bit. This can be done like this:

    ll->wrapIndent = Platform::Minimum(ll->wrapIndent,
    width - static_cast<int>(vstyle.aveCharWidth) * 15);

     
1 2 > >> (Page 1 of 2)

Log in to post a comment.