Editor colour preferences

[gurok]

Rudimentary preferences for editor colours. No property change listeners yet, so the editor(s) must be restarted for colour changes to take effect.

[paulvi]

@gurok It is very nice to get Pull Request. However you haven't talk with anybody in the team. And it seems like you don't know existing issues. Like #15 , so that you could have reference it.

And actually we expect to get support from eclipse-color-theme plugin
eclipse-color-theme/eclipse-color-theme#126

Read README. It was written with purpose

We are going to release 0.4 this week-end. I am not sure if this should be merged. @tomotaro1065

[paulvi]

Looking carefully at code, I disagree. I don't like color preferences on Nodeclipse page.
I also don't like when there are a lot of changed files.

@tomotaro1065 What do you think?

[paulvi]

@gurok And why the heck ExpressProjectWizardPage was changed?
9 changed files. I expected only one

[gurok]

Hi PaulVI,

This is a pull request, not a demand. You and Tomotaro1065 are free to reject it. I just got sick and tired of not being able to use a dark background on my own system.

This is also me making contact with the developers. I apologise for not announcing my intentions first, but to me that seems to be an instruction intended to save my time, not yours. If you are unhappy with this, what is the difference between rejecting a pull request and rejecting a request to make a pull request?

I did read through issue #15 before working on this. How do you reference an existing issue in a pull request?

As for Eclipse Color Theme, I had no idea that you were suggesting a developer try to integrate it. I thought your suggestion was that people use Eclipse Color Theme should they want a solution for dark backgrounds in the interim. You did not make it clear that integration was the intent. I understand this now, but integration like that requires more reading than I'm willing to do. It was enough for me to learn how this project works.

There are 9 files changed, yes. In order to accommodate sensible defaults for colours, I had to modify the names of existing constants. Keeping the same naming convention and simply adding _DEFAULT for the default values is another option, but to me this offers a clear semantic distinction between constants used as keys and constants used as values. Had I not changed the names of those constants, I would estimate a minimum of 3 files would have needed to be changed anyway (PreferenceConstants.java, NodeCodeScanner.java and EditorColors.java).

Why does the number of changed files gripe you? It's not as though there's some limit on the number of files that may be changed. Are you trying to minimise incompatibilities with class loaders that refer to old compiled units or something? I don't see the problem. I'm not arbitrarily renaming things, I'm just trying to improve what's there.

To be honest, this project was a pain to work on. It's not possible to build it normally on my system. I have to manually copy over a "good" copy of NodeEditor.class each time I export. The current configuration doesn't build for me.

This does not need to be and indeed is not intended as a complete and final solution. It's merely better than what was there. I am improving the code, but I am not necessarily perfecting it. This is a proposal with code that's actually written rather than an idea of what could be done. Take it or leave it, I won't be offended.

Best wishes,
Benjamin Penney.

[paulvi]

Hello Benjamin

Welcome to Nodeclipse. You did it your way.

To reference issue in a commit, simply write like #15 . See http://github.github.com/github-flavored-markdown/

I see now, that you actually have considered a lot of things.

Now i propose to merge this after 0.4 release, change a bit and release with 0.5.
That is we should return to this question in a week, after we finish 0.4 testing.

Best wishes,
Paul Verest

http://nodeclipse.github.io/

[tomotaro1065]

Hello @gurok,

We are very sorry for throwing away your efforts, but these changes do not work in the next version 0.4 .
Because we are trying to change NodeEditor to based on JSDT JavaScript Editor in the next version.
So we will not use the following sources in you have changed.

org.nodeclipse.ui/src/org/nodeclipse/ui/highlight/EditorColors.java
org.nodeclipse.ui/src/org/nodeclipse/ui/highlight/NodeCodeScanner.java

Best Regards,
Tomoyuki Inagaki

[gurok]

Yeah, no problem. I look forward to a future version that incorporates colour schemes. At that point, I'll upgrade.

[paulvi]

@gurok @tomotaro1065
Hello Benjamin, Tomoyuki

Having read Benjamin post above again, I found that I overlooked one good point. I also have trouble building.

So Nodeclipse/coffeescript-eclipse#1 considering Tycho, is also actual for Nodeclipse-1.

[paulvi]

I don't see why to close? This pull request should hang until we merge it.

[tomotaro1065]

We are not going to merge it. Because it does not work with JSDT based NodeEditor on ver 0.4.

[paulvi]

In ideal world @gurok could update his fork up to 0.4, and notify again with Pull Request then...
Well, then this pull request is wrong, because it is not yet finished.
So let it be closed.

[paulvi]

@gurok Could you help with eclipse-color-theme/eclipse-color-theme#126

I think there should be color preferences first... We need this Pull Request

[paulvi]

Thanks so much to @gurok , now it is used in Minimalist Gradle Editor