-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Bug fixes (the simplify
branch)
#3943
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
Conversation
src/languages/aspnet.ts
Outdated
@@ -1,55 +1,44 @@ | |||
import csharp from './csharp'; | |||
import markup from './markup'; | |||
import type { Grammar, GrammarOptions, LanguageProto } from '../types'; | |||
import type { Grammar, GrammarToken, LanguageProto } from '../types'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By bad. I transformed this language incorrectly before.
src/languages/aspnet.ts
Outdated
}, | ||
$rest: 'csharp', | ||
grammar ({ base }): Grammar { | ||
const pageDirectiveInside: Grammar = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we rename this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, something is wrong with the changes I propose. I took the language definition from the v2
branch, not the simplify
branch. Let me remove it from the PR for now. We'll return to it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See question but otherwise LGTM
If you are referring to the ASP.NET language and why we renamed the variable, I removed this language from this PR to update it correctly later. |
Summary
extend()