Skip to content

#345 Include option to show completion list after a character is typed #457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 15, 2015

Conversation

mousetraps
Copy link
Contributor

  • trigger completion by default when both AutoListMembers and
    ShowCompletionListAfterCharacterTyped options are selected
  • adds option to options page

Fix #345

…r is typed

- trigger completion by default when both AutoListMembers and
  ShowCompletionListAfterCharacterTyped options are selected
- adds option to options page
@@ -725,8 +732,12 @@ out title
|| (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9');
}

private static bool IsIdentifierFirstChar(char ch) {
return ch == '_' || (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do such helper methods not already exist elsewhere in the parser code?

Also, interesting to see this is all ASCII only (not just your code changes, but surrounding code). Does NTVS not handle Unicode chars? (ES5 allows identifierNames as per the Unicode 3 standard - see http://es5.github.io/#x7.6 ).

@billti
Copy link
Member

billti commented Sep 15, 2015

This will be cool to get in, but changes in these areas are really hard to get just right (just ask @paulvanbrenk how many times he had to redo it for TypeScript 😄 ). Let's not check this in in-between the last RC and RTM.

@@ -193,6 +193,13 @@ internal sealed class IntellisenseController : IIntellisenseController, IOleComm
UpdateCurrentParameter();
}
break;
default:
if (IsIdentifierFirstChar(ch) && _activeSession == null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this when typing in comments, string literals, etc...? I don't see any filtering here on context (e.g. see some of the checks above like on line 169). It's super annoying when the completion list keeps appears when you're typing in places you don't want it (like those just listed). I've seen those bugs before (I may have written a couple of them :-/ )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the other isense code actually blocks that (reverseExpressionParser and VsAnalyzer.) otherwise Ctrl+space would trigger in comments etc.

@billti
Copy link
Member

billti commented Sep 15, 2015

👍

mousetraps added a commit that referenced this pull request Sep 15, 2015
#345 Include option to show completion list after a character is typed
* trigger completion by default when both AutoListMembers and ShowCompletionListAfterCharacterTyped options are selected
* adds option to options page

Fix #345
@mousetraps mousetraps merged commit 0139fe8 into microsoft:master Sep 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants