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

(chore) add name attribute to all language grammars #2407

Merged
merged 6 commits into from Feb 22, 2020

Conversation

taufik-nurrohman
Copy link
Member

@taufik-nurrohman taufik-nurrohman commented Feb 17, 2020

Closes #2394

Related: #2400

Copy link
Member

@joshgoebel joshgoebel left a comment

Choose a reason for hiding this comment

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

Also please remove the extra changelog file you seemed to add.

@@ -197,6 +197,7 @@ export default function(hljs) {
};

return {
name: 'C-like',
Copy link
Member

Choose a reason for hiding this comment

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

This is just a foundation. No name needed.

@@ -19,7 +19,8 @@ export default function(hljs) {
begin: ":\\d{1,5}"
};
return {
aliases: ['apacheconf'],
name: 'Apache',
Copy link
Member

Choose a reason for hiding this comment

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

Please add “config” to match the title.

@@ -86,6 +86,7 @@ export default function(hljs) {

// placeholder for recursive self-reference
var DEFAULT = {
name: 'Julia',
Copy link
Member

Choose a reason for hiding this comment

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

This needs to go in the return block not default.

src/languages/pf.js Show resolved Hide resolved
src/languages/yaml.js Show resolved Hide resolved
@joshgoebel
Copy link
Member

Very nice work. Thanks for the contribution.

@joshgoebel
Copy link
Member

You really should rebase this on master though. I meant I can work with this, but for future contributions the PR will be MUCH nicer and easier to review if you're starting from a clean master.

@joshgoebel joshgoebel changed the title Added name Attribute(s) (chore) add name attribute to all language grammars Feb 18, 2020
@joshgoebel joshgoebel added language parser enhancement An enhancement or new feature labels Feb 18, 2020
@taufik-nurrohman
Copy link
Member Author

Also please remove the extra changelog file you seemed to add.

Done? Huh? Where am I now?

@taufik-nurrohman
Copy link
Member Author

taufik-nurrohman commented Feb 18, 2020

You really should rebase this on master though. I meant I can work with this, but for future contributions the PR will be MUCH nicer and easier to review if you're starting from a clean master.

My first intention was that I wanted to merge this commit onto your branch in yyyc514:lang_has_name. But I just didn’t know how it worked.

Wow, this project is so crowded. I feel lost sometimes.

@joshgoebel
Copy link
Member

lang_has_name is in master now and this commit chain is just crazy.

You should hard reset your own master to upstream master and then cherry-pick your 2 commits back on top. (that's easiest if you don't know how to do the rebase)

@joshgoebel
Copy link
Member

joshgoebel commented Feb 18, 2020

Wow, this project is so crowded. I feel lost sometimes.

Sorry Git and Github can be a real pain if you aren't familiar with them. Have you been merging upstream master into your master (while also committing to your master)? That is bad mojo and will cause you all sorts of problems.

This might be of interest:

https://thoughtbot.com/blog/git-interactive-rebase-squash-amend-rewriting-history

@taufik-nurrohman
Copy link
Member Author

taufik-nurrohman commented Feb 18, 2020

Sorry Git and Github can be a real pain if you aren't familiar with them.

Have been using it for my personal project but never used it to collaborate with others.

Have you been merging upstream master into your master (while also committing to your master)?

That was from my previous pull request. I didn’t delete that repository after my pull has been merged and just continue creating a branch named patch-1.

The commit log data in my patch-1 branch is mixed with yours. Is that normal?

@joshgoebel
Copy link
Member

joshgoebel commented Feb 18, 2020

That was from my previous pull request. I didn’t delete that repository after my pull has been merged and just continue creating a branch named patch-1.

Yeah that's how you get everything messed up. Because the final merge of anything often isn't the same as what's in a PR... so now your history has diverged from upstream master and you start seeing everything twice... to avoid that your master should always be pinned to upstream master.

You can fix it by a hard reset:

# make sure upstream is a remote
git remote add upstream git@github.com:highlightjs/highlight.js.git
# checkout your master branch
git checkout master 
# force reset your master to point to the SAME exact commit as upstream/master
git reset --hard upstream/master

That's just to "fix" master... you should always do your work in branches and NEVER commit to master (when contributing to projects). If you keep your master clean then all you have to do is switch to it periodically and merge upstream:

git checkout master
git merge upstream/master

Once you have your master sane again you can switch to your branch, hard-reset IT to master also and then cherry-pick the two commits you want to get them back. Make sure you write down the commit hashes first (or you can always use git reflog to find them). Or you could do an interactive rebase onto master...

@joshgoebel
Copy link
Member

If that's' too much let me know and I can probably finish this up myself.

@taufik-nurrohman
Copy link
Member Author

Doh, sorry for that extensions part. I should move that to another pull. Please wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature language parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[chore] Moves names of languages into data of grammar itself
2 participants