Skip to content

Loading…

New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + CPFontPanel + RTF parser/producer [+1] #2068

Open
wants to merge 346 commits into from
@daboe01

This is an implementation of the cocoa text system for cappuccino. It is based on the (long-dead) cappuccino-fork from http://github.com/emaillard/cappuccino. I eventually re-wrote most of the code and i.e. replaced canvas-drawing with DOM-spans to address the immanent performance and rendering-quality issues. In my testing the code is currently fully functional. However, Cappuccino still needs two critical mods to make it work:

  • Merge in PR #2045 (CPTextView will crash without it)
  • Pasteboard support is broken in Safari. This has to be worked around somehow to make it useful here.

fixes issue: #1432

@cappbot

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

@Dogild
Cappuccino member

Great job.

You should fixe all of the warnings/errors from capp_lint though ;)

@daboe01

would you be willing to help me with this? ;)

@daboe01

i just fixed the last capp_lint warning/error inside AppKit/CPTextView/
best greetings,
daniel

@Dogild
Cappuccino member

Nice Daniel !

I would like to add some remarks though :

  • I have seen several methods like this - (void)_refreshWithTextView: textView or - (void)setCurrentSize: aSize. It could be great to add the prototype of each params.
  • Some classes implement delegate here. It could be great to call delegate's methods as we do in the other AppKitClass (it's everywhere like this now :)). You can take a look at the PR #2037 for an exemple.
  • There are still some coding style errors, specially with the if (no space, wrong indentation, brackets where there is just one line). Take a look here https://github.com/cappuccino/cappuccino/blob/master/CONTRIBUTING.md#cappuccino-coding-style-guidelines (I would have some times this week end maybe)

Keep going this work ;)

@daboe01

hi alexandre,
thank you for the feedback!
i fixed the missing protoypes+ removed extra spaces from some method calls.
could you please elaborate on the delegation thing?
what exactly needs to be changed?
best greetings,
daniel

@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/CPLayoutManager.j
((209 lines not shown))
+
+ if ([attributes containsKey:CPFontAttributeName])
+ font = [attributes objectForKey:CPFontAttributeName];
+
+ var color = [attributes objectForKey:CPForegroundColorAttributeName],
+ elem = [self createDOMElementWithText:string andFont:font andColor:color],
+ run = {_range:CPMakeRangeCopy(effectiveRange), elem:elem, string:string};
+
+ _runs.push(run);
+ }
+ }
+
+ return self;
+}
+
+- (void)setAdvancements:someAdvancements
@Dogild Cappuccino member
Dogild added a note

Protoype of the param

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/CPLayoutManager.j
((142 lines not shown))
+
+@implementation _CPLineFragment : CPObject
+{
+ CGRect _fragmentRect;
+ CGRect _usedRect;
+ CGPoint _location;
+ CPRange _range;
+ CPTextContainer _textContainer;
+ BOOL _isInvalid;
+ CPMutableArray _runs;
+
+ /* 'Glyphs' frames */
+ CPArray _glyphsFrames;
+}
+
+- (id)createDOMElementWithText:aString andFont:aFont andColor:aColor
@Dogild Cappuccino member
Dogild added a note

Protoype of the params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/CPFontPanel.j
((364 lines not shown))
+ break;
+
+ case kTypefaceIndex_Bold:
+ row = 2;
+ break;
+
+ case kTypefaceIndex_BoldItalic:
+ row = 3;
+ break;
+ }
+
+ [_traitBrowser selectRow:row inColumn:0];
+}
+
+// FIXME<!> Locale support
+- (void)currentTrait
@Dogild Cappuccino member
Dogild added a note

Not a void method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/CPParagraphStyle.j
((124 lines not shown))
+ _alignment = CPLeftTextAlignment;
+ _tabStops = [[[self class] _defaultTabStops] copy];
+}
+
+- (id)init
+{
+ [self _initWithDefaults];
+
+ return self;
+}
+- (id)copy
+{
+ var other = [[self class] alloc];
+ return [other initWithParagraphStyle:self];
+}
+- initWithParagraphStyle:(CPParagraphStyle) other
@Dogild Cappuccino member
Dogild added a note

Protoype of the method + style

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/CPTextView.j
((1,378 lines not shown))
+- (CPDictionary)selectedTextAttributes
+{
+ return _selectedTextAttributes;
+}
+
+- (void)delete:(id)sender
+{
+ [self deleteBackward:sender];
+}
+
+- (CPString)stringValue
+{
+ return _textStorage._string;
+}
+
+- objectValue
@Dogild Cappuccino member
Dogild added a note

Protoype of the method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/_CPRTFParser.j
((310 lines not shown))
+- (BOOL)pushState
+{
+ _states.push["group"];
+ return YES;
+}
+
+- (BOOL)popState
+{
+ _states.pop();
+
+ if (_curState > 0)
+ _curState--;
+ return YES;
+}
+
+- (CPString)_parseSpec:sym parameter:v
@Dogild Cappuccino member
Dogild added a note

Protoype of the params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/_CPRTFParser.j
((419 lines not shown))
+ case "qc": // paragraph center
+ [_currentRun.paragraph setAlignment:CPCenterTextAlignment];
+ break;
+ case "paperw":
+ _paper.width = param;
+ break;
+ case "paperh":
+ _paper.height = param;
+ break;
+ }
+
+ return '';
+}
+
+
+- (CPString)_changeDest:sym
@Dogild Cappuccino member
Dogild added a note

Protoype of the params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/_CPRTFParser.j
((439 lines not shown))
+ _colorArray.push([CPColor blackColor]);
+ break;
+ case "fonttbl":
+ _parsingFontTable = YES;
+ break;
+ }
+ if (sym[4] == "destSkip")
+ {
+ console.log("Dest skip start : [" + sym[0] + "]");
+ _curState++;
+
+ }
+ return '';
+}
+
+- (CPString)_translateKeyword:keyword parameter:param fParameter:(BOOL)fParam
@Dogild Cappuccino member
Dogild added a note

Protoype of the params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPTextView/_CPRTFParser.j
((518 lines not shown))
+ if (_currentRun)
+ {
+ [_currentRun addTab:location type:CPLeftTabStopType];
+ }
+ break;
+ default:
+ console.log("skip : " + keyword + " param: " + param);
+
+ }
+ if (_states.length > 0)
+ _curState = 1;
+ return '';
+ }
+}
+
+- (CPString)_parseKeyword:rtf length:len
@Dogild Cappuccino member
Dogild added a note

Protoype of the params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild
Cappuccino member

Hi Daniel,

Actually when a class implements a delegate/dataSource we are used to use category to call the method of the delegate

For instance in your case we would like to have something like that:

When you call a delegate's method

[self _sendDelegateWillChangeSelectionFromCharacterRange:aRange toCharacterRange:aSecondRange]; 

The category :

@implementation CPTextView (CPTextViewDelegate)

- (void)_sendDelegateWillChangeSelectionFromCharacterRange:(CPRange)aRange toCharacterRange:(CPRange)aSecondRange
{
         if (!(_delegateRespondsToSelectorMask & kDelegateRespondsTo_textView_willChangeSelectionFromCharacterRange_toCharacterRange))
         return;

         [_delegate textView:self willChangeSelectionFromCharacterRange:aRange toCharacterRange:aSecondRange];
}

@end

Hope it's clear, you can take a look of this almost everywhere in the AppKit (CPTableView, CPApplication etc etc).

Other thing who can be great is to implement the protocol CPTextViewDelegate. (https://developer.apple.com/library/mac/documentation/cocoa/reference/NSTextViewDelegate_Protocol/Reference/Reference.html#//apple_ref/occ/intf/NSTextViewDelegate). Obviously we don't have all of the methods, but later I'm sure we will ;)

@daboe01

Hi Alexandre,
Thank you for your help!
I fixed the missing prototypes.

I have some concerns about the delegate refactoring.
Your suggestion obviously makes sense when it as simple as invoking the delegate method if implemented.
However, sometimes the behavior depends on whether the delegate implements a method and we do not always invoke it at that place.
Moreover, the data returned by the delegate often needs processing together with variables local to the calling method before the appropriate ivars can be modified.
I am unsure of whether moving such potentially complex code out of context into a separate method does any benefit. Moreover, this additional indirection costs performance for no good reason, IMHO.
I would advocate to leave the delegate stuff as it is for the moment.
Once merged in, the code will surely be rectified and improved by others.

Best greetings,
Daniel

@daboe01

when travis is happy, i am too :-)

@ahankinson

-#new
+AppKit
+feature
+#accepted
+#needs-improvement

(I'm just tagging it with needs improvement since there's no "under review" status of in the Issue Lifecycle. Thanks for submitting this Daniel!)

Related: #1432

@cappbot

Milestone: Someday. Labels: #accepted, AppKit, feature. What's next? A reviewer should examine this issue.

@daboe01 daboe01 changed the title from New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + font panel + RTF parser/producer to New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + CPFontPanel.j + RTF parser/producer
@daboe01 daboe01 changed the title from New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + CPFontPanel.j + RTF parser/producer to New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + CPFontPanel + RTF parser/producer
@Dogild
Cappuccino member

Hi Daniel,

About the delegate factoring I mentioned, the point is we do it everywhere else like this in Cappuccino. It means it's a bit of pain to look another code who doesn't follow that. Specially when the class are huge like this one ;), for example we did it for the CPTableView and afterwards the code was way cleaner.

I'll have some times in two weeks to look more deeply in this PR.

@daboe01

+1

@daboe01 daboe01 changed the title from New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + CPFontPanel + RTF parser/producer to New: CPTextView, CPTextStorage, CPTextContainer, CPLayoutManager and CPSimpleTypesetter + CPFontPanel + RTF parser/producer [+1]
@cappbot

Milestone: Someday. Vote: 1. Labels: #accepted, AppKit, feature. What's next? A reviewer should examine this issue.

@daboe01

hi alex,
i highly appreciate your help.
let us get this merged in as soon as possible.
best greetings,
daniel

@ahankinson

I can't tell from your commit messages, but are there actual unit tests for this patch (not just manual tests)? With something as big and complex as CPTextView, I would hope that there is a test suite, but I can't see them in this PR.

I'd be happy to help write some, but I'm not sure how to do this in a PR context.

@daboe01

we already have good testing coverage for CPAttributedString which is the major computational basis.

on top of this, some automated tests for keyboard navigation and the RTF parser/producer would probably make sense.

you could fork my cappuccino fork and send me a PR.
this would be automatically ammended to this PR after a merge from my side (if my understanding of github is correct).

best greetings,

daniel

@daboe01

another thing we need is to check is Xcode integration (nib2cib).
i cannot do it myself because i do not use Xcode.

@daboe01

i just commited automated tests for CPTextView copy/paste and keyboard navigation.
i have no idea why travis fails with _ "document" is not defined._

@daboe01

besides the "document" is not defined error, there is another issue:

all my new tests pass locally with the fixedCPAttributedString.j from current master in place.
however, this PR is based on the cappuccino master before the CPAttributedString fix was merged in.

i think i have to rebase this PR to the current master so the tests can pass in travis.

i am not a git pro. can somebody help me with this?

best greetings,

daniel

@ahankinson

Do the tests pass on your machine? Are you running jsc or rhino for your javascript engine?

@Dogild
Cappuccino member

When you use something from the DOM as document you should do something like

#if PLATFORM(DOM)
    var dataURL;

    if ([_filename hasPrefix:@"data:image"])
        dataURL = _filename;
    else if (CPFeatureIsCompatible(CPHTMLCanvasFeature))
    {
        var canvas = document.createElement("canvas"),
            ctx = canvas.getContext("2d");

        canvas.width = _image.width,
        canvas.height = _image.height;

        ctx.drawImage(_image, 0, 0);

        dataURL = canvas.toDataURL("image/png");
    }
    else
        return nil;

    var base64 = dataURL.replace(/^data:image\/png;base64,/, "");
    return [CPData dataWithBase64:base64];
#endif
}
@Dogild
Cappuccino member

To update you PR.

git checkout master
git pull origin (or upstream) master
git checkout thisBranch
git merge origin (or upstream) master

@daboe01

hi andrew,
hi alex,
i actually tried this.
however, locally in GitHub.app, it looked like all recent commits to master became part of my PR.
are these filtered out with synching?
do i need a rebase thing?

@daboe01

hi alex,
thank you for pointing out that the DOM is not available in automated testing. i was not aware of this.
automated testing of CPTextView does not make any sense then.
i will remove the tests in a second commit.
best greetings,
daniel

@daboe01

...but why do the tests pass locally? is this due to jsc instead of rhino?
would it be possible to #ifdef the tests in the testsuite when jsc is there?

@ahankinson

Yeah, I would keep the automated tests but surround them in #ifdefs so that we can run them locally.

@daboe01

unfortunately,
#if PLATFORM(DOM) is false during jake test
any idea on what #if or #ifdef would work?

@aljungberg
Cappuccino member

The DOM is not available during command line testing - it's a browser thing.

That said, with a bit of care, everything that doesn't depend on the DOM can be tested. So for example if the class tracks the cursor and selection position without relying on the DOM, those things can be tested provided the right about of #if PLATFORM(DOM) guards.

@Dogild
Cappuccino member

Hi Daniel, can I get the right to send some commits on your PR please ?

@daboe01

sure. what do i have to do for that?

@Dogild
Cappuccino member

In the settings of your fork

@daboe01

i just added you and @ahankinson.
thank you. did not know about this possibility.
best greetings,
daniel

@Dogild
Cappuccino member

Good it works.

First comment :
CPText should be in an individual file and should get the heritage from CPView and not CPControl (I have no ideas if it's going to break something)

I'll firstly work on the dataSource and delegate refactoring and add the CPTextViewDelegate

@daboe01

we already have a CPText.j in cappuccino.
when using this for CPText, we get a whole bunch of circular imports.

@Dogild
Cappuccino member

So everything should be in CPText.j and we should fixe the circular import

@Dogild
Cappuccino member

Ok, I'll work on the nib2cib and make a manual test with a xib later in the weekend/week

@Dogild
Cappuccino member

Hi Daniel,

I fixed the circular import issue.

What we should really do to :

  • Make some tests from a basic/simple application, as the one I just commit. The things should react as in Cocoa, exactly the same things (behavior, frame, view, color and so on).

This is the code for Obje-c

#import "NUAppDelegate.h"

@implementation NUAppDelegate

- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    // Insert code here to initialize your application

    NSTextView *textView = [[NSTextView alloc] initWithFrame:CGRectMake(0,0,200,200)];
    NSScrollView *scrollView = [[NSScrollView alloc] initWithFrame:CGRectMake(20, 20,220,210)];
    [scrollView setDocumentView:textView];

    [[_window contentView] addSubview:scrollView];
}

@end

Right now, you will see, it doesn't work as expected. We must have the same things and then we can add new feature (nib2cib for instance or new methods).

  • We need to improve the code style, it's quite difficult to read for me (I guess it would be the same for something else)
  • My next step will be to add the CPTextView in the theme Aristo and Aristo2, right now everything is in the init method.
@daboe01

could you please post a screenshot how it should look?
(my macosx box is currently not in a buildable state.)

@Dogild
Cappuccino member
@daboe01

hi alex,

thank you so much for cleaning up and improving my stuff.

i just fixed the background color issue.
the only remaining difference to your cocoa test app is the presence of a verticalScroller.

the reason for this is outside the scope of this PR:
in cocoa the horizontalScroller property of CPScrollView defaults to NO whereas in capp it is YES.

do you see anything else?

best greetings,

daniel

@Dogild
Cappuccino member

I mainly see that when I try to insert something, I have a crash form the first charac^^

@daboe01

i do all dev and test on my standalone framework https://github.com/daboe01/CPTextView that i constantly keep in sync with this PR.
bildschirmfoto 2014-06-04 um 20 20 14

@daboe01

the manual test inside this PR will not work because my CPAttributedString fixes are in other PRs.
we need to get these somehow into this PR.

@daboe01

namely PRs #2107, #2050 and #2045

@Dogild
Cappuccino member

Cool, let's wait the merge of the #2107 so we can merge from the master again.

@daboe01

i just merged current master back into my branch.
i am not a git pro.
we now have a lot of unrelated commits in this PR.
is this looking ok to you?

@aparajita
Cappuccino member

@daboe01 I think you'll have to create a new branch from the latest master and cherry pick the correct set of commits to the new branch.

In the future, when you merge from master use --rebase.

@daboe01

do you mean that i should close this PR and open a fresh one?

@aljungberg
Cappuccino member

You can still rebase onto master without creating a new branch, and that should fix this PR. Merging should have worked too though so not sure why we got into this situation.

Try a rebase and see if that clears things up. Something like this (assuming the official Cappuccino fork is on called upstream in your local git):

git checkout NEW--CPTextView
git fetch upstream
git rebase upstream/master
@daboe01

a revert + second rebase does not seem to do the trick.
sorry, i am stuck.

@aljungberg
Cappuccino member

No need to revert anything, just the rebase. Rebase takes all your local changes and applies them on top of a clean checkout of the upstream master. Now you have created these extra revert commits that can't be removed through a simple rebase, since they're "real" commits unique to your branch.

If you're stuck, just go ahead and make a new branch using the cherry-pick method like Aparajita said. You could force push to your branch in origin to keep the same PR.

@daboe01

git reset --hard 0b2b29b did the trick.
looks much better now.
thank you!

@daboe01

the manual test is now working from within this branch!
best greetings,
daniel
bildschirmfoto 2014-07-13 um 21 07 34

@daboe01

i am aware that we need to move all visual parameters from the init method to this theme thing.
is there anything else that needs to be fixed before this can be merged in?

@Dogild
Cappuccino member
@ahankinson

This PR is getting close. Marking ready-to-commit for review by core dev.

milestone=0.9.8
-#accepted
+#ready-to-commit

@cappbot

Milestone: 0.9.8. Vote: 1. Labels: #ready-to-commit, AppKit, feature. What's next? The changes for this issue are ready to be committed by a member of the core team.

@primalmotion primalmotion commented on an outdated diff
AppKit/CPColor.j
@@ -467,6 +467,16 @@ var cachedBlackColor,
return [[CPColor alloc] _initWithCSSString: aString];
}
++ (CPColor)selectedTextBackgroundColor
+{
+ return [CPColor colorWithHexString:"99CCFF"];
+}
+
++ (CPColor)selectedTextBackgroundColorUnfocussed
@primalmotion Cappuccino member

Typo: should be selectedTextBackgroundColorUnfocused

Anyway, it seems this API are not Cocoa standard, can't you make it private?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@primalmotion primalmotion commented on an outdated diff
AppKit/CPFont.j
((22 lines not shown))
+ return [self _fontWithName:aName size:aSize || [fontDescriptor pointSize] bold:isBold italic:isItalic];
+}
+
+- (CPFontDescriptor)fontDescriptor
+{
+ var traits = 0;
+
+ if ([self isBold])
+ traits |= CPFontBoldTrait;
+
+ if ([self isItalic])
+ traits |= CPFontItalicTrait;
+
+ var descriptor = [[CPFontDescriptor fontDescriptorWithName:_name size:_size] fontDescriptorWithSymbolicTraits:traits];
+
+ return descriptor;
@primalmotion Cappuccino member

can't you directly return it? this assignment seems useless

return [[CPFontDescriptor fontDescriptorWithName:_name size:_size] fontDescriptorWithSymbolicTraits:traits];
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@primalmotion primalmotion commented on an outdated diff
AppKit/CPFontManager.j
@@ -41,7 +44,20 @@ CPUnitalicFontMask = 1 << 24;
var CPSharedFontManager = nil,
- CPFontManagerFactory = Nil;
+ CPFontManagerFactory = Nil,
+ CPFontPanelFactory = Nil;
@primalmotion Cappuccino member

nil not Nil right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@primalmotion primalmotion commented on the diff
AppKit/CPFontManager.j
((36 lines not shown))
+ symbolicTrait |= CPFontItalicTrait;
+
+ if (fontTrait & CPUnboldFontMask) /* FIXME: this only change CPFontSymbolicTrait what about CPFontWeightTrait */
+ symbolicTrait &= ~CPFontBoldTrait;
+
+ if (fontTrait & CPUnitalicFontMask)
+ symbolicTrait &= ~CPFontItalicTrait;
+
+ if (fontTrait & CPExpandedFontMask)
+ symbolicTrait |= CPFontExpandedTrait;
+
+ if (fontTrait & CPSmallCapsFontMask)
+ symbolicTrait |= CPFontSmallCapsTrait;
+
+ if (![attributes containsKey:CPFontTraitsAttribute])
+ [attributes setObject:[CPDictionary dictionaryWithObject:[CPNumber numberWithUnsignedInt:symbolicTrait]
@primalmotion Cappuccino member

alignement. for that kind gf multiline thing, all : should be aligned with one per line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@primalmotion primalmotion commented on an outdated diff
AppKit/CPTextView/CPTextStorage.j
((112 lines not shown))
+ if (![_layoutManagers containsObject:aManager])
+ {
+ [aManager setTextStorage:self];
+ [_layoutManagers addObject:aManager];
+ }
+}
+- (void)removeLayoutManager:(CPLayoutManager)aManager
+{
+ if ([_layoutManagers containsObject:aManager])
+ {
+ [aManager setTextStorage:nil];
+ [_layoutManagers removeObject:aManager];
+ }
+}
+
+- (CPArray)layoutManagers
@primalmotion Cappuccino member

Can't we use @accessors for all of these?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@primalmotion primalmotion commented on an outdated diff
AppKit/CPTextView/CPTextStorage.j
((140 lines not shown))
+}
+
+- (unsigned)editedMask
+{
+ return _editedMask;
+}
+
+- (void)invalidateAttributesInRange:(CPRange)aRange
+{
+ /* FIXME: stub */
+}
+
+- (void)processEditing
+{
+ [[CPNotificationCenter defaultCenter] postNotificationName:CPTextStorageWillProcessEditingNotification
+ object:self];
@primalmotion Cappuccino member

: alignement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@primalmotion primalmotion commented on an outdated diff
AppKit/CPTextView/CPTextStorage.j
((144 lines not shown))
+ return _editedMask;
+}
+
+- (void)invalidateAttributesInRange:(CPRange)aRange
+{
+ /* FIXME: stub */
+}
+
+- (void)processEditing
+{
+ [[CPNotificationCenter defaultCenter] postNotificationName:CPTextStorageWillProcessEditingNotification
+ object:self];
+
+ [self invalidateAttributesInRange:[self editedRange]];
+
+ [[CPNotificationCenter defaultCenter] postNotificationName:CPTextStorageDidProcessEditingNotification
@primalmotion Cappuccino member

: alignement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@daboe01

i just performed extensive testing to ensure this branch works as well as the standalone framework.
it is pretty much finished from my side.
@aparajita is there anything more to do before this can be merged in?
additionally fixes #2357

@Dogild
Cappuccino member

You should resync with the master :)

@daboe01

@Dogild any idea why travis is failing in CPComboBoxTest with a "document is not defined"?
i have not touched anything there!

@Dogild

_window is probably nil here as it's in _init. You should add this mechanism in the private method _addObservers. Take a look to other AppKit classes

@daboe01

thank you, fixed.
however, still not working in my demo.
i will have a look at your manual test this evening.

@Dogild
Cappuccino member

The tests should be fixed now, you should merge the master again. Thx

@Dogild
Cappuccino member

Are you finished @daboe01 ?

@daboe01

yes :-)

@Dogild
Cappuccino member

Hi @daboe01, can you make a new pull request with this feature and a new pull request with one commit ? @ahankinson any objections for that (your commits won't appear anymore) ?

@Dogild
Cappuccino member

HI @daboe01,

Few things to change :

  • When the content of the CPTextView is empty, the position of the cursor is not well positioned. textviewcursor
  • Copy and paste does not work in CPTextView when the CPTextView was created from a xib.
  • Font is different between what you get from the xib and then with what your write.
  • Position of the texts moves when hitting enter at the end of a line textviewenter
  • Position of the cursor is wrong when hitting shift+leftArrow then leftArrow. The cursors should be at the beginning of the word but it is at the end of the previous line textviewcursor
  • In the test CPTextView, when doing cmd+z after loading the application, it erases everything
  • When I redo with cmd+shift+z it select things
  • Not possible to select different parts of the text like in cocoa screen shot 2015-09-25 at 2 07 35 pm
  • Can you create a test with the font panel ?
  • In cocoa, the NSTextView supports in native the cmb+b (bold), cmd+i (italic), cmd+u (underline) and so on.
  • Impossible to select the text just after you have deleted something (pattern -> delete - delete - shift+alt+leftArrow). It only happens when you dot with the last letters of the content. Otherwise in the middle of something it will select something weird.

Keep up the good work ;), we are very close to release it !

@daboe01

thank you for testing.
i opened appropriate issues here: https://github.com/daboe01/CPTextView/issues
i cannot fix the xib related issues because i do not work with Xcode.
multiple selections are unsupported for now as is justified text.

@Dogild
Cappuccino member

Hi @daboe01,

Few other things :

  • When selecting a group of letters and then hitting rightArrow, the cursor is positioned one more space that it should (shift + leftArrow than rightArrow) cursorposition
  • There is not any support of drag and drop of text. See example with the cocoa application. draganddrop
  • Deselection happens with mouseDown, it should be with mouseUp
  • The size of the control increased the first time you add content size
  • The method acceptsFirstResponder of CPTextView is wrong. The textView can't be firstResponder if it is not editable.
@Dogild Dogild commented on an outdated diff
AppKit/CPText.j
((70 lines not shown))
+
+- (void)copy:(id)sender
+{
+ var selectedRange = [self selectedRange];
+
+ if (selectedRange.length < 1)
+ return;
+
+ var pasteboard = [CPPasteboard generalPasteboard];
+
+ // put plain representation on the pasteboad unconditionally
+ [pasteboard declareTypes:[CPStringPboardType] owner:nil];
+ [pasteboard setString:[[self stringValue] substringWithRange:selectedRange] forType:CPStringPboardType];
+
+ if ([self isRichText] && [self respondsToSelector:@selector(textStorage)])
@Dogild Cappuccino member
Dogild added a note

This is very related to the CPTextView. It should be there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild Dogild commented on an outdated diff
AppKit/CPText.j
((46 lines not shown))
+ BOOL _isSelectable @accessors(getter=isSelectable, setter=setSelectable:);
+ BOOL _isRichText @accessors(getter=isRichText, setter=setRichText:);
+}
+
+- (void)setSelectable:(BOOL)flag
+{
+ _isSelectable = flag;
+
+ if (!flag)
+ [self setEditable:flag];
+}
+
+- (void)setEditable:(BOOL)flag
+{
+ _isEditable = flag;
@Dogild Cappuccino member
Dogild added a note

We can bind this ivar. We need to add the willChangeValueForKey and didChangeValueForKey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Dogild
Cappuccino member

It should be just after _isSeletable = flag

@Dogild

It's better to do (for the eyes) :

if (![self isRichText])
return;
@Dogild
Cappuccino member

About the selection (it's not a big things), in Cocoa when some letters are selected and when we click on this same selection, the deselection will be with mouseUp and not mouseDown.

@daboe01
@Dogild
Cappuccino member

I just fixed few bugs with the CPTextView coming from a xib !

Only last big issue from a xib is the copy and paste does not work ! @daboe01 can you take a look to this one ? You can use the CPTextViewCib test, I'm not sure it's related to the command nib2cib, you should be able to reproduce and fix it :)

@Dogild
Cappuccino member

Hi @daboe01,

Few other issue detected :

  • When deleting with backspace, it'll delete one character, then two characters, then one, then two etc etc...
  • Tab seems to not work
  • cmd+z does not work with the CPTextView when loading from a xib
  • cmd+v seems to not work anymore
@daboe01
@Dogild
Cappuccino member

Sorry, my bad I can't reproduce anymore :/ I have some issues with the xib and ElCapitan. I'll try to do some fixes this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.