Skip to content
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

🐛 Fixed a syntax highlight problem #94

Closed
wants to merge 4 commits into from

Conversation

max-programming
Copy link

@max-programming max-programming commented Nov 29, 2020

I fixed an issue that I was facing with the object.property and object.method(). This PR closes #88

@max-programming
Copy link
Author

max-programming commented Nov 29, 2020

Before object.method():

image

After object.method():

image


Before: object.property:

image

After object.property:

image

@max-programming
Copy link
Author

Let me know if any changes are required

@max-programming
Copy link
Author

@simurai Have you reviewed it?

Copy link
Contributor

@simurai simurai left a comment

Choose a reason for hiding this comment

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

@max-programming thanks for the PR. 🙇

What language is this for? JavaScript? Below a quick test how your example renders on github.com:

person.doSomething()
console.log()

person.name
console.memory

and here a screenshot in this theme:

Screen Shot 2020-12-10 at 14 04 44

Hmm.. since the goal of this theme is to stay as close to github.com as possible, I would say that object.method() is already correct. 👍 But object. property could be fixed. But only the property should be blue and the object can remain white.

Does that make sense?

@max-programming
Copy link
Author

@simurai I actually mean that everything works fine when we do person.property or person.method(). But the issue persists when we do document.property or document.method(). I mean any built-in object has the issue. But when we create a custom object, that works fine.

@max-programming
Copy link
Author

image
Here, console.log() is the problem but person.doSomething() is fine.
image
Here, console.memory is the problem but person.name works fine.

Any built-in object has the issue

@max-programming
Copy link
Author

@max-programming thanks for the PR. 🙇

What language is this for? JavaScript? Below a quick test how your example renders on github.com:

person.doSomething()
console.log()

person.name
console.memory

and here a screenshot in this theme:

Screen Shot 2020-12-10 at 14 04 44

Hmm.. since the goal of this theme is to stay as close to github.com as possible, I would say that object.method() is already correct. 👍 But object. property could be fixed. But only the property should be blue and the object can remain white.

Does that make sense?

Yeah I will try to add the blue color to the person.name

@max-programming
Copy link
Author

Also, person.name isn't highlighted on your end because you have not initialized the person variable

@max-programming
Copy link
Author

max-programming commented Dec 21, 2020

@simurai I Did not know that you updated it. I'll see if I can do something else to match it later.
Closing it

@simurai
Copy link
Contributor

simurai commented Dec 23, 2020

@max-programming I Did not know that you updated it.

Sorry, this PR #98 only changes the colors.. as in make it lighter or darker, but doesn't change the logic. Like what should be blue and what should be red etc. So that can still be improved separately.

@cmario92
Copy link

cmario92 commented Feb 1, 2021

any updates on this?
I just noticed that I opened another issue that is related to this, but with JSON key/value pairs #129.

@simurai
Copy link
Contributor

simurai commented Feb 2, 2021

I just noticed that I opened another issue that is related to this, but with JSON key/value pairs #129.

They might be related, but it could also be that each language needs to be defined separately. We can keep #129 open until we take a closer look.

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.

Objects and their properties have same color
3 participants