python-bibtexparser icon indicating copy to clipboard operation
python-bibtexparser copied to clipboard

:bug: align_values allows int or bool, fixes previous min alignment

Open michaelfruth opened this issue 3 years ago • 3 comments
trafficstars

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 ENTRYTYPE entry was considered for calculating the maximum number of characters for entries, which always lead to a minimum value of 9. Now, keys that are not written as BibTeX output are ignored, which leads to an exact computation of the number of characters for entries.

michaelfruth avatar Sep 05 '22 19:09 michaelfruth

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!

MiWeiss avatar Sep 09 '22 06:09 MiWeiss

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?

MiWeiss avatar Sep 14 '22 04:09 MiWeiss

@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)

MiWeiss avatar Sep 22 '22 14:09 MiWeiss

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 :-)

MiWeiss avatar Sep 25 '22 06:09 MiWeiss

Now PR is ready for review again 👍

michaelfruth avatar Sep 26 '22 20:09 michaelfruth