python-bibtexparser
python-bibtexparser copied to clipboard
:bug: align_values allows int or bool, fixes previous min alignment
Fixes #315
Aligns all values according to a given length by padding with single spaces. If align_values is true, the maximum number of characters used in any field name is used as the length. If align_values is a number, the greater of the specified value or the maximum number of characters used in any field name is used as the length.
NOTE:: This commit includes breaking changes: The
ENTRYTYPEentry was considered for calculating the maximum number of characters for entries, which always lead to a minimum value of9. Now, keys that are not written as BibTeX output are ignored, which leads to an exact computation of the number of characters for entries.
Hi @michaelfruth
I have not forgotten this one - and I have also noticed your comments in the other PRs :-)
Regarding this PR: There's an edge case I still need to test & lay out to you. Regarding the other PRs: There's some longer thoughts I have organize in my head and then write down... :-)
I'm quite busy atm, but hope that I can get back with you ASAP. Sorry for the wait - and thanks for your contribution!
Hi @michaelfruth
Excuse my late answer and thanks again for your PR.
There's an edge case of which I am not sure how it should be handled: Assume someone specifies an align_value=x with int x, but there is some key len(key) > x. As I understand it, the current implementation uses len(x), ignoring the user-passed value. As an alternative implementation, I could see that x is applied where possible, and only the too long key "exceeds it", e.g.
@entry{someentry,
lor = "lorem ipsum",
lo = "lorem ipsum",
lore = "lorem ipsum",
loremipsum = "lorem ipsum",
l = "lorem ipsum",
}
I guess this would be more closely to what I'd expect given the naming align_value. Also, it would probably be more useful for large to huge bibtex files, where a single key (like some unexpected value) should not mess up the formatting of the entire output.
What do you think? Any reasons to prefer the current implementation?
@michaelfruth - I guess we'll be able to take this over to v2, hence I recommend we continue with this PR (also merging it into 1.x)
Hi @michaelfruth
Not sure if this (again) ready for review. But in case it is - could you fix the merge conflict first? Would allow to run the actions :-)
Now PR is ready for review again 👍