Open GoogleCodeExporter opened 8 years ago
This is basically a duplicate of bug 52. My most recent in that bug says: The
problem is that it comes down to the internally stored file format for resource
task providers. I'd be happy to accept a patch, but it would need to preserve
the existing format. The UI needs a tweak as well. Contributions welcome!
Original comment by konigsb...@gmail.com
on 2 Jul 2012 at 9:21
I've made some modifications that may go some way towards resolving these
problems. These have been committed here for early feedback:
http://code.google.com/a/eclipselabs.org/r/markbrodziak-workspacemechanic/source
/detail?r=21e1312d2fb4e0ca904847601a60268293d8efa9
There were some issue with the handling of Windows paths, as well as partial
support for variable substitution (it was substituting for single value
settings, but not for Json arrays). I haven't yet had time to take a look at
the UI. I may be able to find some time next week to look at completing this
activity if you are happy with the general direction of these changes.
Original comment by mark.bro...@gmail.com
on 6 Jul 2012 at 2:41
Hey! That's really great of you. Thank you very much.
Please make sure you look at, and electronically sign, the contributor license
agreement (which is mentioned at
http://code.google.com/a/eclipselabs.org/p/workspacemechanic/wiki/DeveloperGuide
)
I am really kind of enjoying being on vacation right now, and doing things like
fix the house, take the dog on long walks, you get it. So I'll probably not
look at this until after the weekend. (Additionally, I'd like to push 0.3 to
the stable repository.)
Original comment by konigsb...@gmail.com
on 6 Jul 2012 at 2:27
Aw, screw it. Here are some comments - mostly surface only - a proper going
through will be necessary.
Tests!! I love tests. You're already my favorite new person of the holiday
weekend.
1. Please replace all the static imports for MechanicPreferences with the
original import. Looks like you had an auto thing.
2. @Rule - I'm still using JUnit 3 (Yes, I know. At least I moved to Mockito.)
What's @Rule?
3. Why can't you make temporaryFolder management automatic? (Why the TODOs?)
4. Why aren't you validating the preferences file in LMPFT.java?
5. There is some reason why I didn't migrate everything to the new preferences
API - some bug. Damn if I can't remember why. Argh.
6. MechanicService:413. s/arg0/event
So here's the concern I have about variable substitution: Let's say I create an
entry like ${x}/y, where x resolves to /x. Then I edit the WM preferences
through the UI, where it shows up as /x/y. When I save that preference back, is
it saved as ${x}/y, or /x/y? The first is preferred, the second is not.
Original comment by konigsb...@gmail.com
on 6 Jul 2012 at 2:51
Hi @Konigsberg, thanks for the review. I'll try to get around to looking at
amending my changes on Tuesday. In the meantime:
1. Yes, I had auto-import set up. I'll sort this out.
2. @Rule is a feature in JUnit 4; the rule has hooks to automatically
execute before and after test methods. So, if we upgrade this test to JUnit
4, the code in the setUp and tearDown methods can go away (hence the TODOs)
- I'm happy to convert this test to JUnit 4 for you if you would like me to.
3. See above.
4. The new preferences API does not seem to have a validation mechanism; it
may be done implicitly - I'm going to try and read up on this and
understand where it happens.
5. Ok - I think we can move back to the old preferences API if necessary,
the main issue here is to make sure that the variable parsing happens in
the right place.
6. Yes, sloppy mistake - sorry about that. It happens sometimes when the
source code for the libraries is not imported into the project.
I share your concern with the variable substitution. I agree that retaining
the variable in the saved preferences is the preferred outcome. I think
that the base level support is still a step in the right direction (so that
values placed in plugin_customization.ini will resolve), but the UI will
need to do the right thing as well. I'd like to have a look at that next
(after these changes are sorted out) - I've never played with Eclipse
plugin UIs before, so it might take me a little while to learn the
different options.
I have another couple of ideas around cleaning up the MechanicPreferences,
but that can come later.
I hope you are enjoying the rest of your holiday weekend...
Mark
Original comment by mark.bro...@gmail.com
on 8 Jul 2012 at 3:20
If you want to put the Mechanic preferences into a separate git branch we can
evaluate it separately.
I think we both agree that this change is a step in the right direction. Since
I get to bugs, code, releases, so infrequently nowdays, I'd rather get it right
the first time. I don't think this is a UI problem, and I think I have a
solution. Let me think about it some.
By the way, did I ask you to electronically sign the CLA? Hmm.. I did. OK. Let
me know when you've done that, please.
Original comment by konigsb...@gmail.com
on 8 Jul 2012 at 1:29
So it seems to me that the solution is to provide an alternative API for
getting and setting task providers without variable substitution. Gotta go,
talk more later!
Original comment by konigsb...@gmail.com
on 8 Jul 2012 at 6:04
I electronically signed the CLA yesterday - perhaps it didn't submit properly?
If this still hasn't gone through after today I'll resubmit.
Original comment by mark.bro...@gmail.com
on 8 Jul 2012 at 10:59
You might have signed it the first time, I just couldn't easily check.
Original comment by konigsb...@gmail.com
on 9 Jul 2012 at 3:04
Note the changes I have made, specifically
http://code.google.com/a/eclipselabs.org/p/workspacemechanic/source/detail?r=bc8
95c8d7217b8a58dbbfaee190f2815411b6b32
and
http://code.google.com/a/eclipselabs.org/p/workspacemechanic/source/detail?r=ce4
cea21630c46f2291e81c47de7d04ff77eeeb1
I believe this will open it up.
Original comment by konigsb...@gmail.com
on 9 Jul 2012 at 4:01
Ok, makes sense. I'll pull your changes across and take a closer look.
What is the project standard baseline for JRE compatibility? The projects are
set up for 'J2SE-1.5' - I downloaded this to cross-check that I hadn't
introduced anything that wasn't backwardly compatible, and there is a single
error in ReconcilingPreferencesTask (using 1.6 String API).
Original comment by mark.bro...@gmail.com
on 9 Jul 2012 at 11:46
The baseline is 1.5.
Original comment by konigsb...@gmail.com
on 10 Jul 2012 at 2:09
Any progress? I'm kind of excited about this being possible.
Original comment by konigsb...@gmail.com
on 12 Jul 2012 at 4:52
Hi Robert,
Sorry, this last couple of weeks have become rather hectic, and as a result
I have not had a chance to rework my changes. I haven't forgotten about it,
but I need a bit of quiet time. I'll let you know when I pick this up again.
Original comment by mark.bro...@gmail.com
on 19 Jul 2012 at 5:40
No problem!
Original comment by konigsb...@gmail.com
on 19 Jul 2012 at 11:07
Is this fixed?
I have this in a task-file
/instance/com.google.eclipse.mechanic/mechanicSourceDirectories=/p/global/mechan
ic_shared_tasks\:${user_homedir}/.eclipse/mechanic
As far as I can tell when testing, it works as intended.
Original comment by walderi...@gmail.com
on 7 Sep 2012 at 11:09
It is not done. user_homedir was supported to begin with, but that's about it.
Original comment by konigsb...@gmail.com
on 7 Sep 2012 at 2:55
Issue 99 has been merged into this issue.
Original comment by konigsb...@gmail.com
on 17 Sep 2012 at 11:28
Original issue reported on code.google.com by
mark.bro...@gmail.com
on 2 Jul 2012 at 3:49