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

Conversation

@MaximSokolov
Copy link
Contributor

Fixes #103

int main() { printf("Hello, world!"); }
{}[]()
;,.

grammars/c.cson Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

language-javascript uses punctuation.terminator.statement for this.

@winstliu
Copy link
Contributor

winstliu commented Dec 2, 2015

What do you think about the differences between this new matching and the one used by language-javascript/other languages?

@MaximSokolov
Copy link
Contributor Author

  1. meta is used to provide context, so punctuation for punctuation is ok.
  2. bracket.{round|square|curly} is correct as the term "braces" is for {,}
  3. I guess separator (special case of "delimiter") is more correct for commas. Not sure about delimiter for the "."
["a", "b", "c" ] // separator
["a", "b", "c",] // delimiter
  1. I use punctuation.other since punctuation.separator/definition/delimiter would be unclear without information about what that punctuation mark separates/defines/delimits (e.g. punctuation.definition.array) Maybe drop other?

@winstliu
Copy link
Contributor

winstliu commented Dec 9, 2015

bracket.{round|square|curly} is correct as the term "braces" is for {,}

This will unfortunately break standardization with other packages. Might not be that big of a deal though depending on how quickly the others are updated and how many themes are using those. @simurai?

I do agree with moving away from meta though.

@simurai
Copy link

simurai commented Dec 11, 2015

From the core themes only Solarized seems to style .brace.

Then probably some forks and people's styles.less overrides. So not too big impact, but still a few.

@MaximSokolov
Copy link
Contributor Author

Only JS, Coffescript, Less tokeize brackets as meta.brace (Search results for "meta.brace" in grammar files) Also isBracket function uses meta.brace (search results for "isBracket") However this function doesn't work properly, since most brackets are tokenized as punctuation.definition.<section>

@MaximSokolov
Copy link
Contributor Author

Changes in 0d8b634:

before:

int fn(...) {...}
   |  ^   ^     |  - punctuation.section.parens.begin/end
   |  ^---^     |  - meta.parens
   ^------------^  - meta.function
{ fn(...) }
 |  ^   |    - punctuation.definition.parameters
 |  |   ^    - no scope
 |  ^---^    - no scope
 ^--^        - meta.function-call

after:

int fn(...) {...}
   |  ^   ^     - punctuation.definition.parameters.begin/end
   |  ^---^     - meta.parameters
   ^------^     - meta.function
{ fn(...) }
 |  ^   ^    - punctuation.definition.arguments.begin/end
 |  ^---^    - meta.arguments
 ^------^    - meta.function-call

Fixes #100

#define ABC XYZ(1)
struct MY_STRUCT {
/* code */
};

Fixes #31

#define EOFCHECK() \
   if(is.eof()) \
    throw std::runtime_error(((std::string) "Setting '") + name + "' not found");

Foo foo(getSomeString() + " (" + otherString + ')');

Fixes #75

#define a >b()
int c;
int d;

Fixes #80

{
    ::printf("Error: MongoDB Invalid Query %s!\n", query ? query->toString().c_str() : "{}");
}

Fixes #82

int connect (const char *address,
            void (*data_recevied)(const char *, const int len)
            void (*on_connect)(),
            void (*on_disconnect)());

Fixes #111

#define LOG1(sev) \
  LOG2(sev) << "#" << " smth "

// everything further is highlighted as string literal
int f(void) {
// smth
}

Fixes this: atom/language-gfm#133 (comment)

}
]
'block':
'begin': '\\{'
Copy link
Contributor

Choose a reason for hiding this comment

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

While we're improving punctuation, let's remove the unnecessary backslashes on curly brackets as well.

'endCaptures':
'0':
'name': 'punctuation.definition.arguments.end.c'
'name': 'meta.arguments.c'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would this be better suited as a contentName? Same for below.

It can be. This allows to style parens inside arguments though:

.meta.arguments {
  & > .punctuation.definition {...}
  // .variable.argument {...}, etc
}
// instead of
.punctuation.definition.arguments {...}
//.variable.argument {...}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's keep it as is then.

grammars/c.cson Outdated
'begin': '\\('
'beginCaptures':
'0':
'name': 'punctuation.definition.arguments.begin.c'
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing .bracket.round on these two

@winstliu
Copy link
Contributor

winstliu commented Feb 5, 2016

Regarding ;:
I just looked and there's absolutely no standardization as to what this is scoped as.
Javascript has it as punctuation.terminator.statement, Java and Go use punctuation.terminator, Ruby uses punctuation.terminator.expression, and Less uses punctuation.terminator.rule.

We should agree on a common scope for this before continuing.

@MaximSokolov
Copy link
Contributor Author

I've fixed unmatched brackets issue by removing begin/end capturing of preprocessor rules.
Everything seems to be working. The only regression:

#ifdef 1 // #ifdef, #ifndef, etc.
#endif
#endif // would be matched as well

This also makes possible to tag unmatched brackets as illegal:
screen shot 2016-02-05 at 14 47 31

Regarding ;

I think punctuation.terminator.statement is correct for imperative programming languages. Not sure about using punctuation.terminator.statement for CSS, Less, etc

@winstliu
Copy link
Contributor

winstliu commented Feb 6, 2016

I think that regression is fine for now. Languages don't have to also be full-blown linters (though it's definitely nice when we can tokenize something as invalid) :P.

'name': 'entity.name.type.cpp'
'captures':
'1':
'name': 'keyword.control.namespace.$2'
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't fix this in this PR, but this capture is missing a .cpp at the end.

'include': '#parens'
}
{
'match': '\\b(const|override|final)\\b'
Copy link
Contributor

Choose a reason for hiding this comment

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

Just making sure...these three will still be matched by function_params, right?

@winstliu winstliu mentioned this pull request Mar 7, 2017
@winstliu winstliu closed this in #212 Mar 7, 2017
@MaximSokolov MaximSokolov deleted the punctuation-improvments branch March 7, 2017 21:08
@MaximSokolov MaximSokolov changed the title Punctuation improvments Tokenize punctuation Apr 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants