Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Undefined words are highlighted as keywords #308

Open
1 task done
wesinator opened this issue Nov 16, 2018 · 7 comments · May be fixed by tree-sitter/tree-sitter-c#16
Open
1 task done

Undefined words are highlighted as keywords #308

wesinator opened this issue Nov 16, 2018 · 7 comments · May be fixed by tree-sitter/tree-sitter-c#16

Comments

@wesinator
Copy link

wesinator commented Nov 16, 2018

Edit by @rsese: from Max in #308 (comment):

It sounds like you want built-in types (like int) to have a different color than user-defined types (like MyLibraryInteger in the example above). I think that makes sense, but I'd like to keep it as a subtly different color (as opposed to a completely different color), because I think there's a lot of value in coloring types consistently for visual understanding of structure.

Prerequisites

Description

any word following a preceding keyword is highlighted as a keyword, even if it is not valid

Steps to Reproduce

Consider the following code:

unsigned int y; // correct

// wrong:
unsigned integer x;
inline integer z;

Expected behavior: 'integer' is not highlighted, as it is not a valid keyword
Words following preceding keyword are only highlighted if they are also actual keywords.

Github and VS Code C highlighting does this correctly.

Actual behavior: any word following a preceding keyword is highlighted as a keyword, even if it is not valid
image

Reproduces how often: Always

Versions

0.60.13
atom 1.32.2

@maxbrunsfeld
Copy link
Contributor

Just to clarify, we're highlighting it as a type, not a keyword. If you use that syntax, integer is indeed a type.

One concrete example is that you can do this:

#ifdef WIN32
#define MyLibraryInteger __int32
#else
#define MyLibraryInteger int32_t
#endif

unsigned MyLibraryInteger MY_LIBRARY_VERSION;

In that case, MyLibraryInteger is a type.

@wesinator
Copy link
Author

Ok, it shouldn't highlight the word unless its a defined type.

The problem is this makes it harder to identify that the user typed an invalid type word if any word is highlighted as a type. The example I gave was a real mistake I had made; it would have been easier to spot if the grammar wasn't automatically highlighting the following word.

@maxbrunsfeld
Copy link
Contributor

It sounds like you want built-in types (like int) to have a different color than user-defined types (like MyLibraryInteger in the example above). I think that makes sense, but I'd like to keep it as a subtly different color (as opposed to a completely different color), because I think there's a lot of value in coloring types consistently for visual understanding of structure.

The reason that this "works" with VSCode and GitHub.com is that those syntax highlighters can only ever recognize built-in types, so user-defined types are generally not highlighted at all.

@wesinator
Copy link
Author

Yes, that sounds good.

Thanks,

@rsese rsese added the triaged label Nov 30, 2018
@wesinator wesinator changed the title Invalid words following a preceding keyword are still highlighted as keywords Undefined words are highlighted as keywords Dec 1, 2018
@wesinator
Copy link
Author

It seems that any undefined word gets highlighted, whether or not it follows an actual keyword
image

This is astonishing behaviour that negatively affects code reading and debugging

@wesinator
Copy link
Author

Workaround: uncheck "Use Tree Sitter Parsers"

Between this and atom/language-todo#82, I'm going to avoid Tree Sitter for the time being.

@maxbrunsfeld
Copy link
Contributor

Hey @wesinator, thanks for raising this issue and providing good examples. The poor error recovery you're seeing in C is a symptom of a missing Tree-sitter feature. I've opened a PR to add that feature (tree-sitter/tree-sitter#246) and in the coming weeks I'll be updating our C parser to make use of it, eliminating this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants