ev3dev / ev3dev-lang

(deprecated) language bindings for ev3dev sensors, motors, LEDs, etc.
GNU General Public License v2.0
56 stars 39 forks source link

Support change of port-name to address in language bindings #137

Closed rhempel closed 8 years ago

rhempel commented 8 years ago

Also removed the Lua binding until it's in better shape.

rhempel commented 8 years ago

@WasabiFan - tag - you're it :-) Just have a look and see if this makes a bit more sense than the previous bungled attempt. Please.

WasabiFan commented 8 years ago

@WasabiFan - tag - you're it :-)

Sorry... had meant to get to looking at this but had gotten sidetracked by shiny :sparkles: objects.

Just have a look and see if this makes a bit more sense than the previous bungled attempt.

The good news? It makes more sense than the previous one. The bad news? Something still seems weird -- but this time it should be easy(er) to fix.

The interesting part is here:

diff --git a/autogen/config.js b/autogen/config.js
index 0deaf23..374df85 100644
--- a/autogen/config.js
+++ b/autogen/config.js
@@ -15,18 +15,11 @@ exports.autogenFenceComments = {
 }

 exports.extraLiquidFilters = {
-    //camel-cases the input string. If the parameter is an array, applies to all items.
+    //camel-cases the input string
     camel_case: function (input) {
-        function camelCaseSingle(input) {
-            return String(input).toLowerCase().replace(/[-|\s](.)/g, function (match, group1) {
-                return group1.toUpperCase();
-            });
-        }
-        
-        if(typeof input == 'string')
-            return camelCaseSingle(input);
-        else
-            return input.map(camelCaseSingle);
+        return String(input).toLowerCase().replace(/[-|\s](.)/g, function (match, group1) {
+            return group1.toUpperCase();
+        });
     },
     //replaces sections of whitespace with underscores
     underscore_spaces: function (input) {
@@ -52,22 +45,6 @@ exports.extraLiquidFilters = {
     eq: function (a, b) {
         return a == b;
     },
-<<<<<<< HEAD
-    prepend_all: function (array, prependStr) {
-        return array.map(function(item) {
-           return  prependStr + item;
-        });
-    },
-    append_all: function (array, appendStr) {
-        return array.map(function(item) {
-           return  item + appendStr;
-        });
-    },
-    select: function(array, property) {
-        return array.map(function(item) {
-           return utils.getProp(item, property); 
-        });
-=======
     //filters the given collection using the provided condition statement, which is
     // evaluated as a JavaScript string (it should return a boolean)
     filter: function(collection, condition) {
@@ -82,6 +59,5 @@ exports.extraLiquidFilters = {
     eval: function (expression, context) {
         var vm = require('vm');
         return vm.runInNewContext(expression, context || {});
->>>>>>> special-sensor-autogen
     }
 };

So, when the previous merge went awry (not the one from the PR, but before that), it added some of that magichery in here and then this is trying to remove some of our previous work.

snip_20151217172512

The more recent of these commits adds those markers (I suspect it's a case of "initiate a merge and forget"), and as a result the conflicts aren't actually resolved -- they're just surrounded by merge markers.

So, I think that if you revert the merge of that PR (both commits) and then redo it, it should be partially fixed. But that still doesn't explain the first bit of the diff above, where the change to autogen so that camel_case can take an array is removed. I can't tell why that is happening... thoughts?

rhempel commented 8 years ago

Sure, I think I can do that too - but not until this evening...

rhempel commented 8 years ago

@wasabifan - do you mean to revert back to [https://github.com/ev3dev/ev3dev-lang/commit/c578f42eee329b54889f678c74773f5a635f6904] ? and then re-apply your PR #127 - then this one?

WasabiFan commented 8 years ago

That's what I am imagining, yes; I hope that'll fix the majority of the issues here.

rhempel commented 8 years ago

OK, I shall make it so ... later tonight

WasabiFan commented 8 years ago

If you'd like me to, I can take a look and see if I can do it -- I'll be at my PC pretty soon, and it shouldn't take more than a few minutes.

rhempel commented 8 years ago

Do I want to revert to that point or reset hard and just blow away those commits? revert keeps history intact while reset gets us back to before I screwed it up. Probably no too many people have cloned the repo since then ...

WasabiFan commented 8 years ago

I think it would be optimal to just do a revert (without actually modifying history). From what I remember there's a special flag that you need to pass to git revert to make it undo a merge... you'll have to figure out the details on that one.

rhempel commented 8 years ago

@wasabifan - I have the repo back in the state where it was before PR#127. Are you available for a PM session?

rhempel commented 8 years ago

That extra code:

        function camelCaseSingle(input) {
            return String(input).toLowerCase().replace(/[-|\s](.)/g, function (match, group1) {
                return group1.toUpperCase();
            });

in exports.extraLiquidFilters was in this commit: [https://github.com/ev3dev/ev3dev-lang/commit/7e098d241e19445eb076e636e2d29846889f6f1b] but not in your special-sensor-autogen branch.

WasabiFan commented 8 years ago

Whoops, got distracted again and forgot to write my response :wink:

Are you available for a PM session?

I assume that's private message? I can get on the IRC channel pretty easily -- I'll do that now and see if I can find you so we can get this sorted there.

WasabiFan commented 8 years ago

OK, well that didn't work very well... I'll try to figure it out and see if I can get the merge to work.

WasabiFan commented 8 years ago

I now believe I understand what is going on.

Basically, when merging the special sensor spec PR, the merge conflicts weren't resolved in the merge. The diff, as we saw before, was checked in raw. This means that the "merge" that had occurred was essentially just deleting all the conflicts and overwriting them. As a result, this is in out Git history and can't be fixed.

There are two ways that I can proceed:

Personally, given that I expect that only a few people have pulled, I would like to take the second option and hard-reset. But this does cause some issues for others, so I'd like to hear from @dlech @ddemidov or @rhempel. My main question is this: what is the work involved for others if I hard-reset? Is there an equivalent to "force push" for pulling?

dlech commented 8 years ago

I would say just force push c578f42eee329b54889f678c74773f5a635f6904 as it was the HEAD before the mess.

git checkout c578f42eee329b54889f678c74773f5a635f6904
git branch -f develop
git checkout develop
git push -f origin develop

In the unlikely event that anyone has cloned the repository in the last couple days, they will have to rebase.

WasabiFan commented 8 years ago

Done. I'll now re-do the PR merge and then manually add the changes from this PR, leavind behind the bad merge. Thanks!

WasabiFan commented 8 years ago

Wow, that was a pretty nasty merge... no wonder it caused so much trouble! :relieved: I just pushed a manually-resolved merge of #127 -- this time it should be non-broken.

Now I'll manually apply the changes from this PR to avoid dealing with this nasty thing as well.

rhempel commented 8 years ago

So everything is merged now and all I have to do is update the python module to point at the right commit in my rhempel/ev3dev-lang/python?

WasabiFan commented 8 years ago

almost; I still need to re-delete the lua binding, but theoretically the port name changes are in there. I will close this PR once I have finished and confirmed that all changes from the PR have been propagated manually. Feel free to make changes now.

Because we modified history, you may have some issues pulling and pushing; you can either manually rebase to get your local copy updated or just re-clone the thing. If you're looking for the former option, you'll probably need to ask @dlech for details. Just make sure that you don't push the commits that were removed again.

Thanks for the port_name changes!

rhempel commented 8 years ago

Here's my plan:

At this point I have got my ev3dev-lang properly in sync with the official one on GitHub. I have set up an upstream remote that points to it so that I can easily keep up to date with the official version.

By the way, thanks for adding the little operators like select to the autogen Liquid filters!

I have a few cosmetic chnages for spec.json, the removal of the Lua binding (for now) and an update to my current Python binding that I can create new PRs for.

I'll do that myself - and I'll be careful to only push in the good stuff :-) But first I need to make some Christmas cookies.

rhempel commented 8 years ago

I'll wait for you to close this PR before I do anything else though!

WasabiFan commented 8 years ago

Here's my plan:

Sounds good!

By the way, thanks for adding the little operators like select to the autogen Liquid filters!

I've just been adding them as I find a need for them -- remember, you can feel free to add them as well! It's just a few lines of JavaScript to implement one.

I'll wait for you to close this PR before I do anything else though!

All the intentional spec changes from this PR should be up there now (tell me if I missed something!). I haven't removed the lua binding yet, but it sounds like you plan to do that, so I'll just close this now and you can make changes as you see fit.

rhempel commented 8 years ago

Awesome. Thks.