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

Character alignment? #6

Open
connermcd opened this issue May 23, 2012 · 8 comments
Open

Character alignment? #6

connermcd opened this issue May 23, 2012 · 8 comments

Comments

@connermcd
Copy link
Collaborator

I love the new custom format options! I do feel like it's lacking some alignment though. It would be great if you could specify a character to align on so that this:

  [    6] José González - Veneer - Deadweight on Velveteen                                    [ 3:27]
  [    7] Erroll Garner - Solitaire - Talk of the Town                                        [ 6:03]
  [    8] Sufjan Stevens - The Avalanche - The Pick-Up                                        [ 3:24]
  [    9] Coldplay - Parachutes - Everything's Not Lost / Life Is for Living                  [ 7:14]
  [   10] Laura Veirs - Troubled by the Fire - Song My Friends Taught Me                      [ 4:32]

would look like this:

  [    6] José González  - Veneer               - Deadweight on Velveteen                     [ 3:27]
  [    7] Erroll Garner  - Solitaire            - Talk of the Town                            [ 6:03]
  [    8] Sufjan Stevens - The Avalanche        - The Pick-Up                                 [ 3:24]
  [    9] Coldplay       - Parachutes           - Everything's Not Lost / Life Is for Living  [ 7:14]
  [   10] Laura Veirs    - Troubled by the Fire - Song My Friends Taught Me                   [ 4:32]

if I set my alignment string to / - / (notice the spaces so The Pick-Up doesn't get aligned).
Would be a great enhancement!

@boysetsfrog
Copy link
Owner

I think something like this is a good idea. I think I would prefer to incorporate it into the format string directly though, but I will see what I can do. Thanks for the suggestions!

@ahti
Copy link

ahti commented Jul 22, 2014

This could be solved similar to how multi-line equations can be aligned in LaTeX, where one may insert a & to mark a place that should be aligned with the corresponding ampersand on the other lines.

@ahti
Copy link

ahti commented Oct 26, 2014

To elaborate a bit, I think specifying an alignment string is a) limiting the possibilities for layout (imagine just aligning things with just a space in between) and b) prone to errors (" - " seems like it could be a common divider, but also occurs in song titles).

@mox-mox
Copy link
Contributor

mox-mox commented Jan 28, 2017

Wheee, I was just searching for some functionality like this.
I really like the approach of having some sort of explicit marker in the songformat string, that says what should be aligned.
But I would also suggest a slightly different approach: Have a marker that says: Up to here should fill up of the line width.
Why?
Two reasons:

  • It looks better. Consider the case where there are only songs with short artist names in the list. Now I insert one song with a long artist name. To keep alignment, the tab stop where all lines are aligned would need to move to accomodate the long artist name. So the screen has to be redrawn and the song titles move to the right. Now, I add the next song...

  • Difficulty of implementation.
    1.) Possibility: Alignment is kept only for the songs currently visible on screen. When scrolling, we would need to re-check, if the tab stop till provides enough space for all artist names. So we would need to re-iterate the visible lines looing for the longes tab stop for each line that the user scrolles. Inefficient and it would lead to the same ugly moving effect described before.
    2.) Alignment could be done globally, fitting all items in the list. It looks good, at least when scrolling, and does not complicate scrolling very much. However, obtaining the longes tab stop is a problem. Or, to be more exact, keeping it. Assume the user adds songs to the list. We can simply compare that songs tab stop requirement (artist name length) and if it is bigger than the current tab stop, update it. But when the user deletes songs, there will be the moment, when the song with the longes tab stop is removed. We would now need to re-scan all remaining songs in the list to find the new longes tab stop. This would turn list removal from O(1) to O(n) complexity. Probably no good idea.

Instead we could add two new tokens to the format string:

  • $A<absolute number> : When printing, the output is padded with ' ' up to the column specified in , then the rest is added. If the window is too small, the text would be cut off where the right-aligned part begins.
  • $P<percentage> : When printing, the output is padded with ' ' until of the window width is filled, then the rest is added. This way, if there are multiple tab stops, each field would receive some clipping if the window is too small.
    $P and $A might not be the best names, it might also be feasible to omit the absolute padding, instead having the P in $P stand for padding or chose a completely different name altogether.

Pros:

  • Minimaly intrusive: Only recognition for the new token in the format string and the padding logic is needed. No additional storage is needed per line or window and runtime complexity class does not change.
  • Nice looking.
  • I might give at least the percentage padding a shot.

Btw: I like the coding style. Not many comments, but well readable :)

@mox-mox
Copy link
Contributor

mox-mox commented Jan 28, 2017

Update: That means, only the parsing function void ScrollWindow::Print(uint32_t line) const
needs an update.

@mox-mox
Copy link
Contributor

mox-mox commented Jan 29, 2017

Side note: I noticed, that using the $A to mean align really was the most intuitive "name" and since I don't really see why one would need absolute alignment, I used that name. However, really any letter can be chosen by changing 'A' to another letter in two places.

@connermcd
Copy link
Collaborator Author

@mox-mox Thanks! On initial testing this appears to work as stated. It fails hard when the specified percentage is <= 9 as stated in the code. Also, when the playlist only contains a few songs with short titles this approach spaces them out quite a bit, but it's definitely a useful contribution.

2017-01-29-081555_1098x673_scrot

@mox-mox
Copy link
Contributor

mox-mox commented Jan 29, 2017

  • About the numbers:
    Numbers smaller than 10 work fine for me, if stated as e.g. $A05.
    I wrote a check to allow a variable number of digits from 0 to 3 but I think this is not really sensible. It adds quite a bit of conditional code at runtime while providing relatively little improvement. Aligning to 100% is not sensible as there would be no space left to put the aligned text and for all other percentages, the two-digit style of writing the numbers is sufficient.

  • About the spacing:
    The spacing is admittedly not so nice and makes it a tad hard to find the remaining part of the line.
    For testing, I used an underscore as fill character which made it easier to follow the line. Maybe having the fill character as a setting would help here?
    Edit: I tested that, and I find it help full :)
    See the updated pull request.

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

No branches or pull requests

4 participants