Code review comment for lp:~zorba-coders/zorba/fn_envvars

- dynamic_context::get_environment_variable(const zstring varname)

should be

dynamic_context::get_environment_variable(const zstring& varname)

- spaces before "(". for example "if (...) instead of if(...)"

- all new functions in sequences_impl.cpp are indented with two spaces. the other functions are not

- if(result)
{
  STACK_PUSH(true, state);
}
STACK_PUSH(false, state);

could be written as STACK_PUSH(result!=NULL, state);

- In FnEnvironmentVariableIterator::nextImpl, the check if(consumeNext(item, theChildren[0].getp(),planState)) is not necessary. consumeNext always returns true because the functions is defined to accept xs:string (not xs:string?)

- item->getString() returns a const zstring&. This should be the type of the variable.

- store::Item_t item2 is not used in the function

- In FnAvailableEnvironmentVariablesIterator::nextImpl, the line a = result->getStringValue().c_str(); and the variable a don't seem to be necessary

- The environment-related functions should be specified using

<zorba:methods>
  <zorba:accessesDynCtx returnValue="true"/>
</zorba:methods>

- All the lines after 14.8.5 fn:unparsed-text have windows line endings

- you can check using transcode::is_necessary if a encoding is supported

- It's unclear when the original stream for unparsed-text is released. This seems to be a memory leak.

- The unparsed-text related functions should be specified as

<zorba:methods>
  <zorba:accessesDynCtx returnValue="true"/>
  <zorba:isSource returnValue="true"/>
</zorba:methods>

- What's the default encoding for unparsed-text?