montegoulding / mergJSON

JSON encode / decode external for LiveCode
http://mergext.com
GNU General Public License v3.0
12 stars 11 forks source link

I forked : here's a pull request #1

Closed mwieder closed 11 years ago

mwieder commented 11 years ago

This is pretty much a do-nothing pull request, just to make sure that things are working. I added a comment to the LIVECODE_ERROR macro and I pushed the changes in external.h even though nothing looks different.

montegoulding commented 11 years ago

Hi Mark, I'm still considering my position on r_err. I know there's been times where I have wished date conversions for example would throw an error. What I might do is rename the macro LIVECODE_THROW_ERROR so it's clear what it's doing. I will also document causes of runtime errors in the functions and how I envisage them being used to handle errors in a command that makes use of the example recursive functions rather than having to modify them to deal with errors. Actually I can't think of a better way to deal with errors in the middle of a recursive function than throwing them. Either way I'm not yet setup with a contributor agreement and as I intend to also sell the external commercially I'm going to close this pull request for now.

montegoulding commented 11 years ago

New plan. What about creating a repo to host external.c/h so we can include it as a submodule? That way we can develop some nicely commented macros that we can all use... For the next few months at least?

M E R Goulding Software development services

mergExt - There's an external for that!

On 17/03/2013, at 1:40 PM, mwieder notifications@github.com wrote:

This is pretty much a do-nothing pull request, just to make sure that things are working. I added a comment to the LIVECODE_ERROR macro and I pushed the changes in external.h even though nothing looks different.

You can merge this Pull Request by running

git pull https://github.com/mwieder/mergJSON master Or view, comment on, or merge it at:

https://github.com/montegoulding/mergJSON/pull/1

Commit Summary

comment re:r_err external.h looks the same File Changes

M mergJSON/external.h (1056) M mergJSON/mergJSON.c (2) Patch Links:

https://github.com/montegoulding/mergJSON/pull/1.patch https://github.com/montegoulding/mergJSON/pull/1.diff

mwieder commented 11 years ago

Monte-

Saturday, March 16, 2013, 11:44:01 PM, you wrote:

New plan. What about creating a repo to host external.c/h so we can include it as a submodule? That way we can develop some nicely commented macros that we can all use... For the next few months at least?

I think RunRev's blessing is needed on this. Aside from the things I added/changed under contract, I believe the files are Scott Raney's doing. ## -Mark mwieder@ahsoftware.net
mwieder commented 11 years ago

Monte-

Saturday, March 16, 2013, 9:42:09 PM, you wrote:

Hi Mark, I'm still considering my position on r_err. I know there's been times where I have wished date conversions for example would throw an error. What I might do is rename the macro LIVECODE_THROW_ERROR so it's clear what it's doing. I will also document causes of runtime errors in the functions and how I envisage them being used to handle errors in a command that makes use of the example recursive functions rather than having to modify them to deal with errors. Actually I can't think of a better way to deal with errors in the middle of a recursive function than throwing them. Either way I'm not yet setup with a contributor agreement and as I intend to also sell the external commercially I'm going to close this pull request for now.

The pull request was really just a test, so there's no problem with closing it. And in lieu of a contributor agreement, I hereby give you permission to do wtf you want with anything I submit to this github repository.

WRT the macro, the name doesn't bother me, it's just the idea that any error will automatically throw an exception, not just return an error from the function. I think two macros are needed: one that really does throw the exception and therefore causes a runtime error that needs to be caught in a try/catch construct; and another that sets r_err=False and just returns an error that can be caught by looking at the result, but isn't serious enough to warrant the throw.

Your point about JSON unraveling being called recursively is a good case for throwing the exception.

-Mark mwieder@ahsoftware.net

montegoulding commented 11 years ago

The pull request was really just a test, so there's no problem with closing it. And in lieu of a contributor agreement, I hereby give you permission to do wtf you want with anything I submit to this github repository.

lol

WRT the macro, the name doesn't bother me, it's just the idea that any error will automatically throw an exception, not just return an error from the function. I think two macros are needed: one that really does throw the exception and therefore causes a runtime error that needs to be caught in a try/catch construct; and another that sets r_err=False and just returns an error that can be caught by looking at the result, but isn't serious enough to warrant the throw.

The second form can't really be used by a function though I guess because it would risk corrupting data if people didn't check for it.

Your point about JSON unraveling being called recursively is a good case for throwing the exception.

Monte Goulding

M E R Goulding - software development services mergExt - There's an external for that!