Add trailer property #1350
Add trailer property #1350
Conversation
The ddt package changed the function signature in version 1.4.3 from idata(iterable) to idata(iterable, index_len). Hopefully this was just a mistake and the new argument will be optional in future versions (see issue datadriventests/ddt#97)
With the command `git interpret-trailers` git provides a way to interact with trailer lines in the commit messages that look similar to RFC 822 e-mail headers (see: https://git-scm.com/docs/git-interpret-trailers). The new property returns those parsed trailer lines from the message as dictionary.
There was a problem hiding this comment.
Thanks a lot for your contribution!
There is only a tiny question about what to do with whitespace in keys. If it's allows, it's worth mentioning it in the docs. Otherwise there could be a test that shows those won't be considered keys, in order to reduce ambiguity.
What do you think about this?
The whitespace handling and trailer selection isn't very trivial or good documented. It therefore seemed easier and less error prone to just call git to parse the message for the trailers section and remove superfluos whitespaces.
| """ | ||
| d = {} | ||
| cmd = ['git', 'interpret-trailers', '--parse'] | ||
| proc: Git.AutoInterrupt = self.repo.git.execute(cmd, as_process=True, istream=PIPE) # type: ignore |
There was a problem hiding this comment.
Can someone help with this?
I don't know how to get mypy to accept subprocess.PIPE as argument
There was a problem hiding this comment.
It looks like the istream type annotation is too narrow in cmd.Git.execute().
It should be int | IO | None if we accept any args that Popen does.
But execute() needs a lot more annotations anyway to clear up the remaining str | bytes problems from py2 and all the overloapping arg type @overloads.
So you can update the type annotation, or leave the type: ignore for now.
Once annotations updated, mypy should then warn that the type: ignore is no longer needed.
|
As I have disabled mypi to not fail CI anymore, I think it's safe to merge this feature and deal with mypi related typing improvements another time. Thanks everyone for their involvement and help, and sorry for the wait. |
|
Should trailers be a list instead of a dictionary? Consider for example this linux kernel commit: https://gitlab.com/linux-kernel/stable/-/commit/1382999aa0548a171a272ca817f6c38e797c458c which has following trailers that include multiple And |
|
I agree - sorry for not catching this. It's better to return it as flat list and leave any kind of indexing to the user. |
Assuming you don't want to make non-backwards compatible change to the API with the fix to this bug then adding a new I've opened #1533 to track this issue. |
Git provides ways to interact trailer lines in the commit messages that look similar to RFC 822 e-mail headers (git interpret-trailers or git log --format="%(trailers)"). The new
trailersproperty of the commit object parses its message and returns this trailer information in a dictionary. This is e.g. usefull when writing a Changelog generator https://docs.gitlab.com/ee/development/changelog.htmlExample commit message:
To access the trailer information you can now use