Skip to content

Ability to set font for JSyntaxTextArea#91

Closed
d0k1 wants to merge 1 commit intoapache:trunkfrom
d0k1:trunk
Closed

Ability to set font for JSyntaxTextArea#91
d0k1 wants to merge 1 commit intoapache:trunkfrom
d0k1:trunk

Conversation

@d0k1
Copy link

@d0k1 d0k1 commented Jan 27, 2016

No description provided.

@milamberspace
Copy link
Contributor

Thanks for your PR.
Some comments:

  • That would be better to use Constants (final public static) for properties strings inside .java files (like jsyntaxtextarea.font, etc.) Particularly with the property key is use into several java files.
  • The change inside the file JMeterUtils.java seems not necessary?
  • Probably the best place inside the jmeter.properties for this new properties is the section Look&feel (start at line ~110)

@d0k1 d0k1 changed the title More JSyntaxTextArea settings Ability to set font for JSyntaxTextArea Jan 28, 2016
@pmouawad
Copy link
Contributor

Hi,
Thanks for PR.
I created https://bz.apache.org/bugzilla/show_bug.cgi?id=58933 for follow up.

@milamberspace I let you handle this.
@d0k1 , if you intend to submit more PRs, it is good to create a bugzilla for it.
Thanks

@d0k1
Copy link
Author

d0k1 commented Jan 29, 2016

Hi! thanks for comments.
I'll make a bugzilla issue by myself, next time.
Could you tell me what should I do next?

@milamberspace
Copy link
Contributor

@d0k1: You can modify your PR to include the comments above directly on your local branch, commit, squash your commits into one commit, push with --force option to update your PR on Github.
After ping me (on github or bugzilla) to allow me to include your code in JMeter source repository.

Copy link
Contributor

Choose a reason for hiding this comment

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

@d0k1 your PR have a bug: when the jsyntaxtextarea.font.size property is defined and jsyntaxtextarea.font.family is not defined, the size is not change on the textarea.
Probably, you must define the default value to "JSyntaxTextArea.getDefaultFont().getName()" the USER_FONT_FAMILY constants, and remove this if condition.

@milamberspace
Copy link
Contributor

@d0k1 don't forget to ping me on github after the update of your PR.

@d0k1
Copy link
Author

d0k1 commented Feb 8, 2016

@milamberspace Thanks for pointing me a bug. I've fixed it, squashed all commits into one.

@asfgit asfgit closed this in 0cde935 Feb 21, 2016
@milamberspace
Copy link
Contributor

@d0k1 Thanks for your improvement patch. Committed on jmeter repo, sorry for long delay for commit.

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.

3 participants