Logger: support of new_line() in the output of the logger

[jvdp1]

A proposition to support new_line properly when inserted in a message as, e.g.,:

call global % log_message( 'Possible issues: ' // new_line('a') // &
 ' 1) First possible issue' // new_line('a') // &
 ' 2) Second possible issue') 

[jvdp1]

I add @wclodius2 as a reviewer

[jvdp1]

@jvdp1:

It looks like you did some revisions before adding me as a reviewer so I am missing some context. My idea, when I suggested using new_line, was to recursively insert new_line in a single character string at the points where we had breaks in write statements, and use that string for a single write to each file. This would have changed format_output_string from a procedure that performs writes to one that returns output string. You seem to be going for a more incremental approach.

All changes can be found here.

I am not sure to understand what was your approach. Sorry.

My idea was to lead the logger writes something like:

ERROR: Possibles reasons:
    1) first reason
    2) second reason

with

call global%log_error('Possible reasons'//new_line('a')//'1) first reason'//new_line('a')//'2) second reason')

Therefore, the user puts some formatting in the message.

Was it your aim too?

[jvdp1]

Here is an additional example of what should be nice to achieve:

• code:

    call global % log_message(&
    'Example of an issue: the new_line() is considered as a character:' //   &
    new_line('a')//                                                                                         &
    ' -1) first bullet point: this is a long sentence on purpose. It should'  //   &
   'be longer than 72 characters;'//new_line('a')                                     //   &
    ' -2) second bullet point' ) 

• Here is the current output (without the proposed changes):

2020-11-01 17:18:03.241: Example of an issue: the new_line() is
    considered as a character:
 -1) first bullet point: this is a long
    sentence on purpose. It should be longer than 72 characters;
 -2)
    second bullet point

• Here is the output with the proposed changes:

2020-11-01 17:23:07.982: Example of an issue: the new_line() is
    considered as a character:
     -1) first bullet point: this is a long sentence on purpose. It
    should be longer than 72 characters;
     -2) second bullet point

[wclodius2]

The output as shown looks very good. My one concern is what might happen if the output unit is asynchronous. In that case the output from different write statements to the same unit might become separated by writes from other parts of the code. This is not a new problem. It was inherent in my original approach. A way around that is to write the output initially to a single string, inserting new_line for the formatting, and then output that string with a single write to each output unit. It becomes a bit tricky with new_line already in the input string, but I think it is doable. Why don't you continue to make the changes you want to get the formatting for your inputs. Then, after you commit your changes to the main branch, I start a new branch to implement the single output string using your mods as a guide for the treatment of new_line in the input string.

[jvdp1]

My one concern is what might happen if the output unit is asynchronous. In that case the output from different write statements to the same unit might become separated by writes from other parts of the code. This is not a new problem. It was inherent in my original approach. A way around that is to write the output initially to a single string, inserting new_line for the formatting, and then output that string with a single write to each output unit. It becomes a bit tricky with new_line already in the input string, but I think it is doable.

This issue was indeed present before the proposed changes. Note that the new_line is already provided by the user in the string. Therefore, I guess it could be considered as a character (as any other ones).

Why don't you continue to make the changes you want to get the formatting for your inputs. Then, after you commit your changes to the main branch, I start a new branch to implement the single output string using your mods as a guide for the treatment of new_line in the input string.

Nice proposition. I believe I finished my changes and it is ready for reviewers. I tested them in a real program with several scenarios, and I was satisfied with the output. Hopefully I covered all possible scenarios.

[wclodius2]

Outside of the two minor comments, the code looks good to me.

[jvdp1]

Outside of the two minor comments, the code looks good to me.

I will wait that other users review the proposed changes too.

[14NGiestas]

Neat change, since length must be less than max_width already.

[milancurcic]

Sorry for the delay, I think this is an improvement, thank you!

[milancurcic]

Please merge if/when ready.

[jvdp1]

Please merge if/when ready.

Thanks for the final review. With 3 reviewers, I will now merge it.

[jvdp1]

I used it mainly in one of our Fortran programs and was in overal pleased by this implementation.

A way around that is to write the output initially to a single string, i

@wclodius2 are you still interested in implementing such a proposition?
While using the logger, I got the following situation:

...some code...
call global%log_message('Initial value is: '//value2char(rdummy)
...some code...
call global%log_message('New value is: '//value2char(rdummy)
...some code...

It would have been nice to have something like:

...some code...
call global%append_to_buffer_message('Initial value is: '//value2char(rdummy))
...some code...
call global%appent_to_buffer_message('New value is: '//value2char(rdummy))
...some code...
call global%log_message()   !write he buffer to file unit(s), and initialise the buffer to''

This is probably further than your initial thoughs

[wclodius2]

Yes it is further than my initial thoughts, which was to make the code thread/asynchronous IO "safe". I could do what you want as part of my rewrite, but what you are proposing has some ambiguities. In the appended strings what would you want done with date/time, module/procedure? What should be the formatting of the appended string?

[jvdp1]

Yes it is further than my initial thoughts, which was to make the code thread/asynchronous IO "safe". I could do what you want as part of my rewrite, but what you are proposing has some ambiguities. In the appended strings what would you want done with date/time, module/procedure? What should be the formatting of the appended string?

I would expect that date/time/module/procedure woud be initialized once when the string is still empty. Additional appended strings would not contain this information. It might be no sense, and could be considered later if others are interested.

At this stage, I think it is more important to have a code thread/asynchronous IO safe.