Add support for setting tabs stops on textboxes

[jirutka]

This allows to emulate a multi-column layout inside the text boxes.

I’m not happy with using the Padding struct for this, but I didn’t find any sensible way how to handle property with multiple distance values. Moreover, the latest Pango release (1.50) added support for tab alignments other than left (right, center and decimal) which would allow even more interesting layouts.

Example:

rofi -modi combi -show combi -combi-modi drun,ssh -combi-display-format '{text}	{mode}'

element-text {
  tab-stops: 500px;
}

[DaveDavenport]

Very nice! thanks.
Would it be worth it to extend the theme parser?

[jirutka]

Would it be worth it to extend the theme parser?

I think so. I’ve wanted to change the parser, but I haven’t found a way to do it without fundamentally modifying it or introducing some weird self-purpose syntax for writing tab stops. Even now the syntax for the border properties is hacked into the “padding” type, so I wanted to avoid another abuse of this type.

If I hadn’t considered these limitations, I would have suggested the following grammar (in EBNF notation):

align = "left" | "right" | "center" | "decimal"
tab-stop = distance, [ align ]
tab-stops = tab-stop, { ",", tab-stop }

Examples:

tab-stops: 50em;
tab-stops: 200px center, 300px, 500px right;

[jirutka]

What I would ideally like is true multi-column layout inside the element-text, i.e. if the text before the tab is wider than the tab size, it does not “push” the text after it to the next tab stop, but shorten it with ellipsis. But this would be too complicated to implement.

[DaveDavenport]

adjusting the parser is not that hard, what we mostly miss is a generic 'list' type.

[DaveDavenport]

I can try to extend the parser so the list type is generic.

[DaveDavenport]

Added a 'testing' set type to git.

element-text {
   tabs:  { 100px, 200px, 300px, 100 em };
}

should be accepted now. It is fully untested.

[DaveDavenport]

Some useful accesors needs to be written for it still.

[jirutka]

Added a 'testing' set type to git.

How do you define the “set”? Set is typically defined as an unordered collection of unique values. If we stick “each distance must be greater than the previous one”, then it can work, but it seems very strange. I’d expect a list, not a set.

Wouldn’t it be better to generalize the existing list syntax ([ 100px, 200px, 300px, 100em ])? Also, how would you implement support for the tab alignment?

[DaveDavenport]

The '[]' syntax was what I wanted to do, but that would break the existing one and would severely complicate the code to get it to work. 'Set' name is not the best name, but the list name was taken.. In the code you get a list. (see the commit msg).

[DaveDavenport]

If this works well, I will see if I can find a work-around for it, but did not want to spend that effort for nothing.

(If it makes you happy I can do a sed rename to array?)

[jirutka]

(If it makes you happy I can do a sed rename to array?)

Yeah, array would be more correct and less confusing name. I’m gonna update this PR.

[DaveDavenport]

Done.. its something intended to be temporary anyway.

[jirutka]

Something is wrong, I’m getting NULL from rofi_theme_get_array_distance and I dunno why yet.

[DaveDavenport]

I just added a test, that worked.

[DaveDavenport]

rofi_theme_parse_string("element-text  { tabs: { 10, 20px, 30px, 40px };}");
  ck_assert_ptr_nonnull(rofi_theme);
  // ck_assert_ptr_null ( rofi_theme->widgets );
  ck_assert_ptr_null(rofi_theme->properties);                                                                                                                                            
  ck_assert_ptr_null(rofi_theme->parent);                                                                                                                                                
  ck_assert_str_eq(rofi_theme->name, "Root");                                                                                                                                            
  GList *l = rofi_theme_get_array_distance(&wid, "tabs");                                                                                                                                
                                                                                                                                                                                         
  ck_assert_int_eq(g_list_length(l), 4);                                                                                                                                                 
                                                                                                                                                                                         
  int i = 10;                                                                                                                                                                            
  for (GList *iter = g_list_first(l); iter != NULL; iter = g_list_next(iter)) {                                                                                                          
    RofiDistance *d = (RofiDistance *)iter->data;                                                                                                                                        
    ck_assert_int_eq(d->base.distance, i);                                                                                                                                               
    i += 10;                                                                                                                                                                             
  }                                                                                                                                                                                      

  g_list_free_full(l, g_free);                     

that worked.

[jirutka]

It works when I do the following change:

 GList *rofi_theme_get_array_distance(const widget *widget,
                                      const char *property) {
- ThemeWidget *wid2 = rofi_theme_find_widget(widget->name, widget->state, TRUE);
+ ThemeWidget *wid2 = rofi_theme_find_widget(widget->name, widget->state, FALSE);

Every other accessor in theme.c passes FALSE to this function.

[DaveDavenport]

What is your theme entry?

[jirutka]

What do you mean by theme entry?

[DaveDavenport]

what did you set in your theme you are trying to match?

[jirutka]

Aha, this:

element-text {
    tab-stops: { 100px, 200px };
}

[DaveDavenport]

Yeah, looking at it it should probably be FALSE.
This is still (partly) inherited to how we did inheriting in the past (people found inheriting confusing).

I should really clean this up.

[DaveDavenport]

updated.

[jirutka]

Done.. its something intended to be temporary anyway.

What will be the longer-term solution?

[DaveDavenport]

My plan is (but I need to give it some more thought) to merge 'list' and 'array'.
So it is:

tabs: [10px,20px,30px];

but I need to see how I resolve the element names here, but I think I have a solution that keeps things clean-ish.

[DaveDavenport]

The {} was for you to be able to continue, while i thought about this one.

[jirutka]

What about tab alignments (left, right, center, decimal)? I see this as the main motivation for changing the syntax, otherwise “padding” would be ok-ish.

[DaveDavenport]

We can add that now more easily, but I dont want to update to pango 1.5 yet (I have no machine with that yet)
.

[jirutka]

We can add that now more easily

👍

(I have no machine with that yet)

Me neither, it’s not even in Alpine Linux Edge yet, but we are already working on it. It will be in upcoming v3.16 which should be released between May and June.

[DaveDavenport]

Thanks!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.