Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/text alignment #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cowile
Copy link

@cowile cowile commented Aug 21, 2019

Hello,

I attended the DFW Elixir Hack Night on 08/20 hosted by some of the codedge team. It was my first introduction to Elixir, but I liked what I saw and read most of the getting started guide later that night.

As thanks for hosting the event, I wanted to contribute something, so I have implemented cell-level text alignment for your pretty-printed tables. It uses a global format_opt currently. A more advanced feature could change alignment based on the type of data to cover a common convention of left-aligned text and right-aligned numbers, but I have other things on my plate considering offers to change jobs.

- Add `alignment` parameter to `cell` function to control text alignment to be left, right, or center.
Add `alignment` field to `format_opts` type to configure alignment passed to formatter code.
@hpopp
Copy link
Member

hpopp commented Aug 24, 2019

Thanks for this! Cell alignment has been on my roadmap, so this is a great first step.

I noticed a bug with center alignment. I suspect it's probably rounding errors with integer division.

iex(2)> Scribe.print(data, alignment: :center)
+---------------+---------------------------------------+------------------+--------+-------------------+
|  :__struct__  |                :email                |   :first_name   |  :id  |    :last_name    |
+---------------+---------------------------------------+------------------+--------+-------------------+
|     User     |       "kasandra2089@spinka.com"       |   "Henriette"   |  4776  |    "Mitchell"    |
|     User     |      "adolfo.moore@treutel.info"      |     "Treva"     |  1497  |     "Ziemann"     |
|     User     |        "frances2011@wolf.org"        |    "Giovanny"    |  1629  |     "Rolfson"     |
|     User     |       "karine1937@douglas.com"       |     "Baron"     |  9890  |    "Powlowski"    |
|     User     |        "arturo2038@johns.info"        |     "Ryder"     |  5487  |    "Gottlieb"    |
|     User     |      "fletcher1910@wunsch.info"      |     "Lukas"     |  1610  |    "Gleichner"    |
|     User     |      "tania.stanton@mueller.biz"      |      "Faye"      |  9632  |      "Huels"      |
|     User     |      "ollie_krajcik@pfeffer.com"      |     "Angel"     |  7773  |       "Von"       |
|     User     |         "brenda2055@kuhn.com"         |     "Roman"     |  3920  |     "Steuber"     |
|     User     |        "cierra1995@blick.info"        |     "Leslie"     |  8423  |     "Wiegand"     |
|     User     |     "dewayne_schaden@hermann.org"     |   "Gabrielle"   |  2252  |      "Yost"      |
|     User     |     "mekhi_keeling@mueller.info"     |     "Blanca"     |  8764  |      "Stehr"      |
|     User     |     "maximilian1939@fritsch.net"     |     "Alysa"     |  3150  |     "Dibbert"     |

For `:center` alignment, add back possible lost space due to integer division.
@cowile
Copy link
Author

cowile commented Aug 25, 2019

You are correct. More specifically, integer division of an odd number of padding spaces will lose one space. This commit should fix by calculating any remainder spaces needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants