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

Support For Annotations in Java #905

Merged
merged 1 commit into from
Mar 8, 2016
Merged

Conversation

desertjim
Copy link
Contributor

  1. Adding annotation token
  2. Making the annotation token match before function

@zeitgeist87
Copy link
Collaborator

Hi @desertjim

Thanks for contributing. Annotations are indeed missing in Java.

Your PR contains a lot of minified files, that have nothing to do with Java annotations. What minifier did you use to create these?

You either have to add the css class .token.annotation to all themes, or use the alias property:

'annotation': {
    pattern: /(@{1}\w*\(.*?\))|(@{1}\w+)/i,
    alias: 'regex'
}

Isn't @{1} equivalend to just @?

@desertjim
Copy link
Contributor Author

Hi @zeitgeist87

I was just using gulp. I thought it was minifying the js files. Here is what I did:

    1. reset the mini files (except prism-java.min.js)
    1. added .token.annotation to all the themes
    1. update the regex to just use @ instead of @{1}
    1. tested it with all the themes, looks good

@zeitgeist87
Copy link
Collaborator

I was just using gulp. I thought it was minifying the js files.

gulp is correct. Maybe you are using an older or newer version of gulp-uglify. You could try executing npm update before you call gulp.

  1. tested it with all the themes, looks good

prism

I am not sure if the colors you chose are such a good idea. They are very bright and prominent. My Eclipse standard theme for Java uses a more subtle grayish color for annotations. Intellij uses some kind of olive green. I am not sure how other IDEs and themes style annotations:

eclipse

idea

We should probably use a color, that is more familiar to most Java developers. What do you think? Do you have a particular reason for choosing bright orange?

The groovy language already has a simpler regex for annotations:

Prism.languages.insertBefore('groovy', 'function', {
    'annotation': {
        pattern: /(^|[^.])@\w+/,
        lookbehind: true
    }
});

Also you don't need the capturing groups and the case insensitive flag. This should be enough: /@\w*\(.*?\)|@\w+/. Do we really need to capture the annotation parameters @{1}\w*\(.*?\)?

@desertjim
Copy link
Contributor Author

I just installed gulp-uglify so I must be using a newer version.

I do like the color choice, but my aim was to not end up having code directly adjacent to the annotations being the same color. So I just picked an existing color to make sure it was still in the same theme and went with it. To avoid bikeshedding I'll leave the color to whatever you suggest :).

I can update the regex to match groovy, should I use the lookBehind: true also(sorry not familiar with this)?

@zeitgeist87
Copy link
Collaborator

To avoid bikeshedding I'll leave the color to whatever you suggest :).

Well I am not really sure what the best choice is here. Here are a few examples:

  • Netbeans

netbeans

  • Stackoverflow

stackoverflow

  • Github
public class Duck extends Base {
    @Override
    public void quack() {
        System.out.println("Quack");
    }
}

I am open for suggestions, but in my opinion annotations should not stand out more than for example keywords.

I can update the regex to match groovy, should I use the lookBehind: true also(sorry not familiar with this)?

Yes lookbehind is necessary.

@desertjim
Copy link
Contributor Author

How about this? Still in the theme, darker and not the same color as surrounding text:
how about this

@LeaVerou
Copy link
Member

LeaVerou commented Mar 6, 2016

What about aliasing it to punctuation? Then it would look discreet in all themes.
(just an idea)

@zeitgeist87
Copy link
Collaborator

What about aliasing it to punctuation? Then it would look discreet in all themes.
(just an idea)

Yes let's do that. I think it looks nice. We can always change it later:

alias

@desertjim You could also add alias: 'punctuation' to the Groovy language while you're at it. Thanks!

@desertjim
Copy link
Contributor Author

@zeitgeist87 should be good now

@zeitgeist87
Copy link
Collaborator

@zeitgeist87 should be good now

Thanks! It looks good to me. You should also squash down all of your commits into a single commit. Otherwise we get all the back and forth into the PrismJS git log after the merge.

2)Aliasing java annotation to punctuation
3)Aliasing grovy annotation to punctuation
4)Adding in sample code
@desertjim
Copy link
Contributor Author

ok, it's ready

zeitgeist87 added a commit that referenced this pull request Mar 8, 2016
Support For Annotations in Java
@zeitgeist87 zeitgeist87 merged commit 5978d4a into PrismJS:gh-pages Mar 8, 2016
@just-boris
Copy link
Contributor

Is this going to be released?

@LeaVerou
Copy link
Member

We make releases periodically, but it's already in Prism. Everyone downloading Prism from the website can get it. :)

@just-boris
Copy link
Contributor

I am installing this from NPM and it seems that it doesn't have this feature. Could you please make a release for it?

@LeaVerou
Copy link
Member

We do not make releases for specific features, we release periodically. This will be in the next release, which will happen soon.

@chenbihao
Copy link

How about this? Still in the theme, darker and not the same color as surrounding text:这个怎么样?仍然在主题中,颜色较暗且与周围文本的颜色不同: how about this

oh does not feel so good now, which will cause comments // and annotations @ to confuse

like this:

image

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.

5 participants