patchGNU Octave - Patches: patch #9059, reformatting ov-java.cc?

 
 

patch #9059: reformatting ov-java.cc?

Submitter:  Ernst Reissner <ernstreissner>
Submitted:  Wed 27 Jul 2016 09:09:26 AM UTC
Votes: 1
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 15 Aug 2016 10:32:02 PM UTC, comment #8: 

I pushed this change with a modified commit message and after stripping out trailing whitespace.

http://hg.savannah.gnu.org/hgweb/octave/rev/0448bf26fe98

I don't agree that these changes make the file much more readable.  The real problem is that many expressions include long function names, reinterpret_cast<TYPE>, and long literal character strings.  But I committed it anyway so that maybe it will help you move forward.

John W. Eaton <jwe>
Group administrator
Mon 15 Aug 2016 09:50:52 PM UTC, comment #7: 

for #9067 jwe wrote:
'Please try to avoid mixing style changes with other substantive changes. It makes the substantive changes harder to evaluate. '

For this, 9059 I tried right this:
patch formatting only.
The result is: no review at all...

I would like to encourage you considering this patch,
rejecting or not,
because it makes subsequent changes more comfortable.

Ernst Reissner <ernstreissner>
Thu 11 Aug 2016 03:17:11 PM UTC, comment #6: 

Hi Mike,
Thank you for considering my improvements on java sources.
As you saw, I readily made bugfixed thereafter.

Now the same with ov-java.cc.

I prepared bugfixes for 48750, 48759, 48653,...
and am waiting for a review of ov-java.cc.

It is really just reformatting and a review shall not need much time.

I will provide docs as bugfixes by and by.


Ernst Reissner <ernstreissner>
Wed 10 Aug 2016 11:11:18 PM UTC, comment #5: 

I need feedback to the way i reformatted the source.
Also I would like to go on bugfixing
based on this more readable source.
So this shall be reviewed first.

Ernst Reissner <ernstreissner>
Wed 03 Aug 2016 08:59:16 PM UTC, comment #4: 

ok now I added a file with moderate reformatting.

(file #38097)

Ernst Reissner <ernstreissner>
Wed 27 Jul 2016 01:25:43 PM UTC, comment #3: 

yes,
i suggest this only in the given case
because the lines are very long (much longern than 80)
in this case.
Is it ok, to place a function argument in the next line
if it becomes longer than 80 lines,
otherwise go on with the current line?

ok, i got it: no comments on } and #endif.


Ernst Reissner <ernstreissner>
Wed 27 Jul 2016 01:14:42 PM UTC, comment #2: 

There's no need to go to the extreme of placing each function argument on a separate line.

Please don't comment "}" or #endif.

John W. Eaton <jwe>
Group administrator
Wed 27 Jul 2016 11:01:18 AM UTC, comment #1: 

Ok, the same again with proper formatting:


octave_value
octave_java::do_javaMethod (void *jni_env_arg,
                            const std::string& name,
                            const octave_value_list& args)
{
#if defined (HAVE_JAVA)

  octave_value retval;

  JNIEnv *jni_env = TO_JNIENV (jni_env_arg);

  if (jni_env)
    {
      jobjectArray_ref arg_objs (jni_env), arg_types (jni_env);
       if (oct2javaType (jni_env, args, arg_objs, arg_types))
        {
          jclass_ref helperClass (jni_env,
                                  find_octave_class (jni_env,
                                                     "org/octave/ClassHelper"));
          jmethodID mID = jni_env
            ->GetStaticMethodID (helperClass,
                                 "invokeMethod",
                                 // parameter types
                                 "(Ljava/lang/Object;Ljava/lang/String;"
                                 "[Ljava/lang/Object;[Ljava/lang/Class;)"
                                 "Ljava/lang/Object;");
          jstring_ref methName (jni_env, jni_env->NewStringUTF (name.c_str ()));
          jobject resObj = jni_env
            ->CallStaticObjectMethod(helperClass,
                                     mID,
                                     to_java (),
                                     jstring (methName),
                                     jobjectArray (arg_objs),
                                     jobjectArray (arg_types));
          // FIXME: why array???? bug?
           jobjectArray_ref resObjArr (jni_env,
                                      reinterpret_cast<jobjectArray> (resObj));
           retval = resObjArr
            ? java2octType (jni_env, resObjArr)
            : check_exception (jni_env);
         } // if (oct2javaType

      octave_set_default_fpucw ();
    } // if (jni_env)

  return retval;

#else

  octave_unused_parameter (jni_env_arg);
  octave_unused_parameter (name);
  octave_unused_parameter (args);

  // This shouldn't happen because construction of octave_java
  // objects is supposed to be impossible if Java is not available.

  panic_impossible ();

#endif
}


Ernst Reissner <ernstreissner>
Wed 27 Jul 2016 09:09:26 AM UTC, original submission:  

I would like to provide a patch
reformatting ov-java.cc to make it more readable
in particular, sticking to 80 chars per line.

I feel in particular, that invocations of jni
are not properly formatted.

An example how to improve:

octave_value
octave_java::do_javaMethod (void *jni_env_arg,
                            const std::string& name,
                            const octave_value_list& args)
{
#if defined (HAVE_JAVA)

  octave_value retval;

  JNIEnv *jni_env = TO_JNIENV (jni_env_arg);

  if (jni_env)
    {
      jobjectArray_ref arg_objs (jni_env), arg_types (jni_env);
       if (oct2javaType (jni_env, args, arg_objs, arg_types))
        {
          jclass_ref helperClass (jni_env,
                                  find_octave_class (jni_env,
                                                     "org/octave/ClassHelper"));
          jmethodID mID = jni_env
            ->GetStaticMethodID (helperClass,
                                 "invokeMethod",
                                 // parameter types
                                 "(Ljava/lang/Object;Ljava/lang/String;"
                                 "[Ljava/lang/Object;[Ljava/lang/Class;)"
                                 "Ljava/lang/Object;");
          jstring_ref methName (jni_env, jni_env->NewStringUTF (name.c_str ()));
          jobject resObj = jni_env
            ->CallStaticObjectMethod(helperClass,
                                     mID,
                                     to_java (),
                                     jstring (methName),
                                     jobjectArray (arg_objs),
                                     jobjectArray (arg_types));
          // FIXME: why array???? bug?
           jobjectArray_ref resObjArr (jni_env,
                                      reinterpret_cast<jobjectArray> (resObj));
           retval = resObjArr
            ? java2octType (jni_env, resObjArr)
            : check_exception (jni_env);
         } // if (oct2javaType

      octave_set_default_fpucw ();
    } // if (jni_env)

  return retval;

#else

  octave_unused_parameter (jni_env_arg);
  octave_unused_parameter (name);
  octave_unused_parameter (args);

  // This shouldn't happen because construction of octave_java
  // objects is supposed to be impossible if Java is not available.

  panic_impossible ();

#endif
}

Ernst Reissner <ernstreissner>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #38097:  patch9059.patch added by ernstreissner (25KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ernstreissner (I would like to fix this before i do further work))
  • -email is unavailable- added by ernstreissner (Voted in favor of this item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by ernstreissner (Submitted the item)
  • -email is unavailable- added by ernstreissner
  •  

    There is 1 vote so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-08-15 jwe StatusNone Done
        Open/ClosedOpen Closed
    2016-08-10 ernstreissner Carbon-Copy- Added -email is unavailable-
    2016-08-09 ernstreissner Carbon-Copy- Added ernstreissner
    2016-08-03 ernstreissner Attached File- Added patch9059.patch, #38097
    2016-07-27 ernstreissner Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code