Multi line event split

[HackingM]

Hi,

I think it would be useful if gitchangelog were able to process multiple events from the same commit, like the example below...

new: Added a feature
fix: Fixed a bug

...should result in two entries to the changelog, one for the new feature and one for the bug fix.

The code I have added achieves this, although I am sure you could have done a much better job!

Basic documentation has been added to gitchangelog.rc.reference.

I would be interested to hear your views - please be kind about my Python, I'm a C++ dev at heart!

Thanks.

[vaab]

May I ask you for a complete example of the content of a commit message containing 2 "events" with a body ?. If you have some corner cases (no body in one or the other commit.), please include them also. Depending on that I'll take the inclusion decision.

At first glance in your code, it seem that you do not allow to have a body for your inner events, and I don't understand why, as it doesn't seem so difficult to do based on your code.
I'm not sure I want to break the formatting rule of commit message about having one line as summary followed by a blank line, then the body. And I do not think that we need to break it to achieve your goals.

In general these are my reflection on this topic:

• In one hand, it's good to have some constraint (here, one addition per commit) to enforce more generic coding clarity. (one commit should be the smallest concern possible)
• In the other hand, it happened quite often that I needed to put several "event" in one commit. But either it was lazyness from my side or multiple minor and cosmetics changes. However, I won't be against loosening a little bit this constraint and leave the dev understand that he should not use this so often as:
  • the inner events won't be displayed in standard oneline git log.
  • the inner events won't trigger some functionality (as commit hooks on github).
• But, I clearly see one specific use of have 2 commit message in one when having to speak to 2 different audience. For instance, having a user bug fixed (then text targets the user: Button is now red), and wanting to add some details for the developper ("color_button() is now generic and handle case X and Y."). This is usefull when creating different changelog, one recording technical changes for developper and one for the end-user recording usage changes without code references.

I've read your code and here are some further remarks (any modification of your code can wait until I make myself clear about an inclusion). So these are more general remarks as much for myself than for your curiosity about python:

• You broke the integration tests.
• You probably didn't want to push the .gitignore but if you had this in mind, please check PR new: dev: Untrack typical files #15
• Not sure about the "event" new term, if it'll be kept, it'll need definition in the README.rst
• I don't fancy long variable name, but split_regex might not be as clear that I wished to. split_event_regex might suffice.
• you probably have a bug line 891, you wanted to write: events = [commit.subject]
• Having a default split_regex value defined, it seems to make little sense to check (len(strip(split_regex)) > 0), and even if split_regex is empty or not found in the summary, it'll output an list of one element which would perfectly fit the follow processing.
  • Why strip split_regex ?
  • besides, these would be more pythonic:
    • without surrounded parenthesis, python do not requires them contrary to C++
    • without > 0 as INT > 0 as same truth value than INT (False if 0)
    • without len(...) as len(STR) as the same truth value than STR (False if empty)
    • OPTIONAL sidenote: use split_regex.strip() rather than strip(split_regex)
    • you can also use the one line form of the if condition which is very close to the ternary C/C++ operator COND ? VALUE_IF_TRUE : VALUE_IF_FALSE. It's python syntax is : VALUE_IF_TRUE if COND else VALUE_IF_FALSE.
• When having a if condition in a for loop that hold the entirety of the for body, I usually invert the condition and issue a continue, this allows to reduce indentation level of the whole body and I find it's easier to read.
• with all my remarks, it's probably possible to reduce your diff size to a few lines (5-10 lines)
• for final inclusion, test are missing as are docs in README.rst

Many thanks for your interest and your proposal.

[HackingM]

Hi Vaab,

Thank you for looking at my suggested modifications.

I have made the changes you suggested (more for my own sake than for yours) and it is a great improvement.

To answer your request for a complete example of the content of a commit message containing 2 "events" with a body, I do not have such a commit! All the commits I tested it on are multiple lines of subject with no body.

With this in mind I would like to add a new section "Notes" to the output which contains numbered entries (the body of the commit message) referenced by the entries split from the subject. Clearly, this would be more complex as if only a single "event" is present in the subject this is not needed and the body could just be quoted as before. I have now changed my code to simply throw away the body when using a split_event_regex. Sadly my ideal is probably beyond my current level of Python-fu! I would need to cull any ignored events and then, if there was still more than one add a note with multiple references or if there is only one use the previous behaviour.

To back up my idea it is exactly the situation where one has two commit message in one when having to speak to two different audience which prompted my modifications.

Also with regard to the code - it now passes the integration tests! I have changed the default behaviour back to the original default behaviour by setting the default value for split_event_regex to None and removing the erroneous default (which I used when testing). I have also fixed the regex to properly split on entries (it needed to match the : too!).

Thanks for your interest in my proposal - and the free Python class! :-)

[coveralls]

Coverage decreased (-0.35%) to 91.77% when pulling d40870f on MADhacking:multi_line_event_split into 32e77a2 on vaab:master.

[coveralls]

Coverage decreased (-0.35%) to 91.77% when pulling d40870f on MADhacking:multi_line_event_split into 32e77a2 on vaab:master.

[coveralls]

Coverage decreased (-0.35%) to 91.77% when pulling d40870f on MADhacking:multi_line_event_split into 32e77a2 on vaab:master.

[coveralls]

Coverage decreased (-0.35%) to 91.77% when pulling d40870f on MADhacking:multi_line_event_split into 32e77a2 on vaab:master.

[coveralls]

Coverage decreased (-0.35%) to 91.77% when pulling a3482a4 on MADhacking:multi_line_event_split into 32e77a2 on vaab:master.

[vaab]

What do you think if we allowed the full commit message to be splitted upon a newline, followed by a new summary ?

This would allow this type of commit message::

fix: usr: remove exception about ``bar`` when fizzing the foo.

This exception happened only on plateform X and Y and is now
fixed for every user.

fix: dev: removed typo in ``fiz()`` that casted a ``bar`` exception when ``wiz`` is ``None``.

This changes the API of ``fiz()``, and a defensive approach was taken.

Or even (first without body):

fix: usr: remove exception about ``bar`` when fizzing the foo.

fix: dev: removed typo in ``fiz()`` that casted a ``bar`` exception when ``wiz`` is ``None``.

This changes the API of ``fiz()``, and a defensive approach was taken.

Or even (second without body):

fix: usr: remove exception about ``bar`` when fizzing the foo.

This exception happened only on plateform X and Y and is now 
fixed for every user.

fix: dev: removed typo in ``fiz()`` that casted a ``bar`` exception when ``wiz`` is ``None``.

This changes the API of ``fiz()``, and a defensive approach was taken.

Or several with only their summary::

fix: usr: remove exception about ``bar`` when fizzing the foo.

fix: dev: removed typo in ``fiz()`` that casted a ``bar`` exception when ``wiz`` is ``None``.

Tags, and all the other mecanism would apply.

This doesn't seem to be very difficult to implement, and could leverage your split_event_regex...

By the way, I think "entry" is much better than "event", as we are speaking of logs, and that seems to be the consecrated vocabulary.

Please, feel free to share your thoughts about these suggestions...

[HackingM]

Thanks for your feedback.

Firstly, I chose the name event instead of entry because each git log entry is already referred to as an entry (obviously) and we are talking about multiple events logged as a single entry. I was trying to avoid the confusion of referring to multiple entries encapsulated in a single entry. Of course, it is your project so you should have final say on this.

Secondly, I would still like to implement two subjects with one body (as our company git policy would not allow for commits with subjects that would not show up in a summary). I give an example below.

fix usr: this would be the first entry
fix dev: this would be the second entry

This would be a shared body referenced as a note where needed or in-line when filtering
reduces this to a single occurrence.

This would allow for four possible types of event/entry, and presumably any combination thereof:

1. Single subject no body
2. Single subject with body
3. Multiple subject no body
4. Multiple subject with body

With regards to the note in the above example it could be displayed as per the existing code when referenced from a single event and as a numbered note when referenced from multiple (post filtering) events.

[vaab]

Ok, I understand your point for the terminology choice between "entry" and "event".

It seems that your way of separating events in a commit message is rather local to your company's requirement. There a several point where the format chosen is bad in a general point of view as it goes against standards (one line summary then new line), and is quite limited compared to a the other proposed choice.

However, I understand that this is your need, and this might be a good reason to have an option in the configuration file that allow to separate a commit message to commit event. This options should be callable taking the full commit message and outputing multiple sub commit messages. This would allow a direct function to be used, but we could of course try to simpilfy the writing of this like it was done for body_process or subject_process.
Like this you could implement your company's need and any other people could also implement their need. The value in the sample reference config file (which provide an opiniated way to format a changelog) could then implement what I proposed.

[coveralls]

Coverage decreased (-0.6%) to 91.525% when pulling 9f8b2ee on MADhacking:multi_line_event_split into e9759c2 on vaab:master.

[LuisAlejandro]

I also would like a split functionality like this. I wonder what is left to make this in a mergeable state.