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

core: add short_text support [wip] #1494

Closed
wants to merge 4 commits into from

Conversation

cyrinux
Copy link
Contributor

@cyrinux cyrinux commented Sep 3, 2018

Hi guys,

I started this branch few months ago, this is an @ultrabug request 😉
I prefer open the PR before forget the tasks.
I just rebased on master, fix travis complain.

The goal is to support the short text feature which shrink text when no more space is available.

When I done the branch, this was working, but it is very simple implementation.

What we have for the moment (if i remember correctly my work):

  • max_size = true , if true will auto shrink if needed to display max info.
  • max_size = value will shrink all format to the value size.
  • short_format per module, which define a cleaner choosen format if no space avaible.

for eg, wwan module:

        response = {
            'cached_until': self.py3.time_in(self.cache_timeout),
            'full_text': self.py3.safe_format(self.format, wwan_data),
            'short_text': self.py3.safe_format(self.short_format, wwan_data)
        }

its config:

wwan {
...
short_format = '[\?color=state state ]'
...
}

Now few questions, need help to progress.
As you see, short_text if defined in the module.
How to implement this for all module without pass on all? I can batch the edit of all module but is it the best way to do?

To be continued...
If you have advise before i continue (or not?).
edit: sorry I have noise if files changed (search "short" ^^)

@lasers lasers added the enhancement 😍 I have created an enhancement label Sep 3, 2018
@tobes
Copy link
Contributor

tobes commented Sep 3, 2018

@cyrinux

Yeah this is something that has bugged me for a long time.

This branch is a mess with lots of random commits which makes it hard to review - could you try to clean it up so that only your commits are visible.

If you get stuck I'll try to help out.

@cyrinux
Copy link
Contributor Author

cyrinux commented Sep 3, 2018

Yes will try to clean this one and continue on it. I will for sure need you help, will ask you ;)

@cyrinux
Copy link
Contributor Author

cyrinux commented Sep 3, 2018

@tobes branch cleaned ;)

@tobes
Copy link
Contributor

tobes commented Sep 3, 2018

thanks I'll try to review it soon

@tobes
Copy link
Contributor

tobes commented Sep 9, 2018

max_size I think this name is not very clear, I'm not sure of a better one.

I don't think we should do anything in core.py that feels the wrong place. The ere colour stuff there should probably be moved at some future point.

There are two different things going on.

  1. the module provides a short format.
  2. we create a short format from the module output

I think we should just start with (1) and get that nice. Then we could think about (2) I'm not sure that 2 is such a good idea anyway.

@cyrinux
Copy link
Contributor Author

cyrinux commented Sep 10, 2018

Ok @tobes will clean this PR to simplify it so and support short_text without magic thing for the moment.

@lasers
Copy link
Contributor

lasers commented Oct 2, 2018

Plz resolve teh conflicts. Thx, mate.

@cyrinux
Copy link
Contributor Author

cyrinux commented Oct 2, 2018

Hey, I have not only conflict but thing to remove in. I dont find time those weeks to work on.

simple way to transform with settings a 'max_size' globally for all module or per module.
short_text is used if returned text size is larger than monitor.
@cyrinux
Copy link
Contributor Author

cyrinux commented Oct 2, 2018

@lasers can you come on irc?

@cyrinux
Copy link
Contributor Author

cyrinux commented Oct 2, 2018

green but not working, hands up, will restart from scratch

@ultrabug
Copy link
Owner

@cyrinux should I close it then and you start fresh?

@tobes
Copy link
Contributor

tobes commented Oct 10, 2018

I'd like a simpler implementation that only has short_format.

That keeps things much simpler as there is actually quite a lot of complication when you look at the details of possible outputs. For such a simple feature it is remarkably tricky and that is before we start asking difficult questions about some of our modules.

This is actually one of the many features that I've implemented in the past but never made a PR

@ultrabug ultrabug mentioned this pull request Dec 30, 2018
@ultrabug
Copy link
Owner

Thanks again @cyrinux but this PR is too big and contains unrelated stuff. I'm gonna close until you or someone else takes on this general short_text support

I liked this PR to the #1584 TODO

Cheers

@ultrabug ultrabug closed this Dec 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 😍 I have created an enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants