davibe / Phonegap-SQLitePlugin

A phonegap plugin to open and use sqlite databases on iOS.
191 stars 90 forks source link

Added Cordova 1.5 Support #23

Closed coomsie closed 12 years ago

coomsie commented 12 years ago

Hi,

I've added Cordova 1.5 Support.

I've also renamed a few files as per other peoples suggestions ...

Cheers Iain

brodybits commented 12 years ago

@coomsie this looks like very good work and I definitely plan to try this later today. I do have a few more suggestions:

- What about people using "legacy" versions of "PhoneGap": do we try to keep a way to support them or do we just say "sorry"?

brodybits commented 12 years ago

and also there is an Android plugin that I believe is following the HTML5 Web SQL API: https://github.com/apache/incubator-cordova-android/blob/master/framework/src/org/apache/cordova/Storage.java; I think it would be nice if we could return an object that follows the same API for the iOS version of Cordova/[PhoneGap].

brodybits commented 12 years ago

I just added pull request #24 to propose a change to the API to come closer to the HTML5/W3 SQL API and the existing Android Storage module. I hope you will consider the changes for the Cordova version.

coomsie commented 12 years ago

yeah, fine have added ... https://github.com/davibe/Phonegap-SQLitePlugin/pull/24 changes to latest commit ... https://github.com/coomsie/Cordova_SQLitePlugin/commit/4b7b18107212637ed972b9d71a15295d85534a04

will add to this branch too ..

brodybits commented 12 years ago

And one more patch for README.md to fix the CoffeeScript example:

$ git diff
diff --git a/README.md b/README.md
index 29ca527..696f2a7 100644
--- a/README.md
+++ b/README.md
@@ -59,7 +59,7 @@ General Usage

     db.transaction (tx) ->

-      tx.executeSql [ "INSERT INTO test_table (data, data_num) VALUES (?,?)", "test", 100], (res) -
+      tx.executeSql "INSERT INTO test_table (data, data_num) VALUES (?,?)", ["test", 100], (res) ->

         # success callback

@@ -67,7 +67,7 @@ General Usage
         console.log "rowsAffected: #{res.rowsAffected} -- should be 1"

         # check the count (not a part of the transaction)
-        db.executeSql "select count(id) as cnt from test_table;", (res) ->
+        db.executeSql "select count(id) as cnt from test_table;", [], (res) ->
           console.log "rows.length: #{res.rows.length} -- should be 1"
           console.log "rows[0].cnt: #{res.rows[0].cnt} -- should be 1"

Thanks @coomsie

brodybits commented 12 years ago

@coomsie I looked again and you missed the changes to src/sqlite_plugin.coffee and src/lawnchair_sqlite_plugin_adapter.coffee

coomsie commented 12 years ago

ta, done.

On Wed, Mar 28, 2012 at 9:25 PM, Chris Brody < reply@reply.github.com

wrote:

And one more patch for README.md to fix the CoffeeScript example:

$ git diff diff --git a/README.md b/README.md index 29ca527..696f2a7 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ General Usage

    db.transaction (tx) ->
  • tx.executeSql [ "INSERT INTO test_table (data, data_num) VALUES (?,?)", "test", 100], (res) -
  • tx.executeSql "INSERT INTO test_table (data, data_num) VALUES

    (?,?)", ["test", 100], (res) ->

      # success callback

    @@ -67,7 +67,7 @@ General Usage console.log "rowsAffected: #{res.rowsAffected} -- should be 1"

      # check the count (not a part of the transaction)
  • db.executeSql "select count(id) as cnt from test_table;", (res) ->
  • db.executeSql "select count(id) as cnt from test_table;", [], (res) -> console.log "rows.length: #{res.rows.length} -- should be 1" console.log "rows[0].cnt: #{res.rows[0].cnt} -- should be 1"

Thanks @coomsie


Reply to this email directly or view it on GitHub:

https://github.com/davibe/Phonegap-SQLitePlugin/pull/23#issuecomment-4750758

Cheers Coomsie

brodybits commented 12 years ago

You still missed the changes to the source files src/sqlite_plugin.coffee and src/lawnchair_sqlite_plugin_adapter.coffee

coomsie commented 12 years ago

Ok ta,

I'm not a coffee user. Will change tonight if I have time.

Cheers Iain

Sent from my iPhone 4

On 29/03/2012, at 12:55 AM, Chris Brodyreply@reply.github.com wrote:

You still missed the changes to the source files src/sqlite_plugin.coffee and src/lawnchair_sqlite_plugin_adapter.coffee


Reply to this email directly or view it on GitHub: https://github.com/davibe/Phonegap-SQLitePlugin/pull/23#issuecomment-4757433

brodybits commented 12 years ago

@coomsie sorry to bother you but I got a comment from someone that a couple more changes are necessary to follow the WebSQL API. I expect to submit the changes sometime later today.

coomsie commented 12 years ago

b.t.w.

HTML iOS 5.1 web sql changes will effect phonegap/cordova and make this a bit more important ....

check this out

brodybits commented 12 years ago

Awesome. You know that they already have a solution that does backups and restores upon app startup and shutdown but I do not like this in case an app or even a device is crashing in the middle of running an application. I am working on a solution right now to provide a consistent API between PhoneGap 1.2/1.3, Cordova 1.5, and in the future for the Android.

brodybits commented 12 years ago

I have reorganized my fork to contain versions for both PhoneGap (1.3-) and Cordova (1.5+), with plans to add an Android version in the future. It is in the following location: https://github.com/chbrody/Cordova-SQLiteNative

I made changes to db.transaction() and also to the callbacks in tx.executeSql() in the Legacy-PhoneGap-iPhone subdirectory only so far and have asked someone else to take a look. If they are OK then I plan to patch them into the Cordova-iOS version as well.

davibe commented 12 years ago

I am thinking, does it make sense to maintain support for older phonegaps ? I think we should onle support "the current stable".

+1 for Android version.

brodybits commented 12 years ago

I would agree however some people may be stuck with an older PhoneGap until almost all of the plugins are ported to Cordova 1.5+. In addition, this is a very nice fix for people with an older PhoneGap who are having problems with the iOS 5.1 Web SQL API changes (check this out from @coomsie)

davibe commented 12 years ago

I still think people using older phonegap should just git reset to an older revision of this plugin. BTW I am finally working to port some projects to cordova so I will look at this patch and merge it soon.

brodybits commented 12 years ago

Well someone is actually testing and using my API changes in an older PhoneGap so I do want to keep it in my branch for the time being. I have pushed quite a few API changes and I leave it up to you whether or not you want to take these.

brodybits commented 12 years ago

Also I had an idea to drop db.executeSql() in favor of using the transaction, since db.executeSql() is not according to the HTML5/W3 SQL API and I don't really want to add this to the Android version. Any comments?

coomsie commented 12 years ago

i suggest going with the latest supported stable version, people can go back via git.. to get the older version if need be.

also lets try to keep it as close the to the spec as possible ...

project might in future change again, when the new web db is common place .... chrome seems to be pushing ahead in that regard.

marcucio commented 12 years ago

We have a version of this that works well on 1.4 (thanks chbrody) so why not put it out there. There probably are many people like myself who cannot upgrade their projects yet to 1.5 because many of the plugins are not on 1.5. Here is the 1.4 list of plugins:

https://github.com/phonegap/phonegap-plugins/tree/master/iPhone

compared to the list of 1.5 plugins:

https://github.com/phonegap/phonegap-plugins/tree/master/iOS

brodybits commented 12 years ago

I have renamed my project (again) to https://github.com/chbrody/Cordova-SQLitePlugin/ and added a DroidGap version, use it at your own risk!

brodybits commented 12 years ago

I have made some changes to test the Android (DroidGap) version with the Lawnchair test suite, and I have now made a common Lawnchair adapter that works with both the iOS and Android versions. The Android and Cordova-iOS versions use sqlitePlugin.openDatabase() to open a database to work closer to the Web SQL API. I want to test and incorporate some fies from @marcucio then add a pull request if this is working OK.

davibe commented 12 years ago

I decided to update the plugin to make it work with Cordova/Phonegap 1.5.0 with minimal modifications. I think we should keep it simple and just support the current release of Phonegap. Users can always use git to roll back to an older version. I am going to add git tags to help users decide. I did not change the name from Phonegap to Cordova yet, i think i will.

I love the idea of having an html5-like api, but this should not break the old one. Is there any test suite we can use to implement the api properly?

It's nice to see someone working out the android side too. I suggest to create a separate git project for that.

brodybits commented 12 years ago

In that case, I will keep my changes separated but still as a "fork" so I hope you will give @coomsie the credit for the 1.5.0 fixes. I have been doing the testing by using an adapted version of the sample from your website together with the basic testsuite from Lawnchair. I do want to dump the old PhoneGap junk from my project but one of my goals was to keep the same API between iOS and Android and I want to continue in this direction. @davibe thanks again for this work, it has helped a number of people who were affected by the iOS 5.1 changes.