vicoapp / vico

Mac Programmers Text Editor
http://www.vicoapp.com/
657 stars 89 forks source link

Block/line comment trigger #58

Closed peteschaffner closed 11 years ago

peteschaffner commented 11 years ago

I was just noticing that the standard cmd+/ keybinding for commenting out a line, seems to have a glitch. It appears that the action is firing, as you can see a brief flash of the appropriate comment characters being inserted, but then those characters are immediately removed.

Also worth noting is that with the option modifier (for inserting a block comment), the comment is inserted, but not removed like with the line comment. However, it seems the action is fired twice, as a second block comment is nested inside the first.

The offending filetypes that I have noticed this behavior in are CSS and JavaScript.

I hope this is enough info to reproduce the bug!

Thanks for the wonderful work on this editor btw ;)

Shadowfiend commented 11 years ago

Interesting. Cmd-/ is actually firing three times judging by how many undo hits it takes to undo the changes. I'll investigate when I get a chance.

peteschaffner commented 11 years ago

Good catch! Thanks so much for looking into it.

Shadowfiend commented 11 years ago

I appreciate your filing the bug :) Just yesterday I was thinking “there must be a way to comment a block…” but I couldn't find it, so you also taught me something hahaha.

peteschaffner commented 11 years ago

Haha nice!

peteschaffner commented 11 years ago

I just noticed that triggering the "auto close tag" action in HTML files also fires the action twice ... so it seems to me to be an issue with how TextMate text actions are interpreted/invoked. I will look into this and see if I can find any more correlation.

Shadowfiend commented 11 years ago

Try this branch on for size. I think it fixes the issue at hand.

peteschaffner commented 11 years ago

I'm having difficulties building and I wonder if its related to me upgrading to Xcode 5 :\ This is the error I'm getting:

Check dependencies
error: There is no SDK with the name or path '/Users/pete/Work/Personal/Vico/source/sparkle/macosx10.7'

** BUILD FAILED **

The following build commands failed:
    Check dependencies
(1 failure)
make: *** [/Users/pete/Work/Personal/Vico/source/build/DEBUG/sparkle/Sparkle.stamp] Error 65

Damnit I wanted to try this out so bad haha!

Shadowfiend commented 11 years ago

Hmm. Try changing sparkle/Configurations/ConfigCommon.xcconfig, line 6, to refer to macosx10.8 instead. Can't guarantee that'll work.

Just to double-check, you are running make in the vico directory, right? Rather than attempting to build from XCode? While we're getting closer to building successfully in XCode, it's not quite there yet.

peteschaffner commented 11 years ago

No luck. Yeah I am still using make to build (and am in the root directory when running it) ... I also have initiated all submodules so I know its not that. I tried this patch which seemed to fix the Sparkle issue, but then I got a couple assigning block literal to a weak variable errors:

app/SFTPConnection.m:1283:16: error: assigning block literal to a weak variable;
      object will be released after assignment
      [-Werror,-Warc-unsafe-retained-assign]
                innerReadfun = ^(SFTPMessage *msg) {
                             ^ ~~~~~~~~~~~~~~~~~~~~~
app/SFTPConnection.m:1489:17: error: assigning block literal to a weak variable;
      object will be released after assignment
      [-Werror,-Warc-unsafe-retained-assign]
                        innerReadfun = ^(SFTPMessage *msg) {
                                     ^ ~~~~~~~~~~~~~~~~~~~~~
app/SFTPConnection.m:1641:17: error: assigning block literal to a weak variable;
      object will be released after assignment
      [-Werror,-Warc-unsafe-retained-assign]
                innerWritefun = ^(SFTPMessage *msg) {
                              ^ ~~~~~~~~~~~~~~~~~~~~~

Sorry for all the trouble :P I wish downgrading Xcode wasn't such a bitch.

Shadowfiend commented 11 years ago

I thought I might get screwed on that heh. Lemme put a fix onto master for that issue.

Shadowfiend commented 11 years ago

See if that helps. It may not, as I'm not running XCode 5 yet at the moment, and therefore can't double-check if I've properly satisfied the compiler :/ If not, I'll have to have a look tomorrow.

peteschaffner commented 11 years ago

No cigar. Damn.

Thanks for trying! I will be around all tomorrow so we can look at it then. I wish I could be of more help here ... but unfortunately Obj-c isn't my area :)

Shadowfiend commented 11 years ago

That one seems to compile for me.

peteschaffner commented 11 years ago

Works for me too ... and your comment trigger fixes work like a charm as well (both single line and block)! You are the man!

Shadowfiend commented 11 years ago

Awesome! We'll get 'er merged right in then, thanks for double-checking :)