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

Discussion about merging the two project? #36

Open
casouri opened this issue Jul 7, 2018 · 10 comments
Open

Discussion about merging the two project? #36

casouri opened this issue Jul 7, 2018 · 10 comments

Comments

@casouri
Copy link

casouri commented Jul 7, 2018

I just saw your new branch, and it’s pretty cool! I’d happy to merge my project to yours if we can agree on the code. I haven’t investigate every line but already have some idea based on your README. 😄

@Yevgnen
Copy link
Owner

Yevgnen commented Jul 7, 2018 via email

@casouri
Copy link
Author

casouri commented Jul 8, 2018

Your update is quite complete and I don't think I can add too much to it, except this: (I'm guessing)

ensure that the candidate always have enough space

screen shot 2018-07-08 at 2 53 51 pm

Here you can see that candidate "sssssssssssssssssssssssssssssssssssssssssssss" took space from the part following it.

Also, I configure the length of each part by using a proportion to the max width instead of a fixed length.

@Yevgnen
Copy link
Owner

Yevgnen commented Jul 9, 2018

Thanks for the details. I'll take a look the implementation details of ivy-filthy-rich later.

The candidate width issue is a bit annoying. In the older version of ivy-rich, long path like /aaaaaaa/bbbbbbb/ccccccc/ddddddd/eeeeeee/fffffff/ggggggg.el that will be shorten into /aaaaaaa/.../ggggggg.el. However, in the customize branch, the max width and the column format function (which only take the candidate as a single argument). I currently have no idea how to make each column always display with reasonable width.

@Yevgnen
Copy link
Owner

Yevgnen commented Jul 9, 2018

Also, thanks for mentioning ivy-rich in ivy-filthy-rich.

@casouri
Copy link
Author

casouri commented Jul 9, 2018

However, in the customize branch, the max width and the column format function (which only take the candidate as a single argument).

What about the max width and format function?

About width issue, to just provide some ideas, in my implementation I use "info-function"s to provide information. For example, ivy-filthy-rich--get-path returns the path of the buffer. However, those functions don't return a string, they return a list of strings, ranging from longest to shortest.

My transformer will then first try to fit the longest string into a column; if it fails, it tries to use the next one. If the last one still doesn't fit, the transformer will simply truncate the string. Note that all this happens before the candidate eats up anyone's place.

I currently have no idea how to make each column always display with reasonable width.

I'm not sure if I understand you, because there is a :width spec in the format, shouldn't that just locks everyone in its place?

@Yevgnen
Copy link
Owner

Yevgnen commented Jul 9, 2018

Transforming a candidate to a list of candidate string for a transform function seems a bit complicate for users... 😅

I like the idea of ivy-filthy-rich--concat-entry-sequence which allow column overwriting. My original thought is to always shorten/trim every value using ... from the right once they are longer than the max width defined in ivy-rich--display-transformers-list. Maybe we can provide an option to this.

However, this works not so well with file paths since if shortening is necessary, one would usually prefer /aaaaaaa/.../ggggggg.el to /aaaaaaa/bbbbbbb/.... The file path format function does not know whether the length of the candidate is longer than the max allowed value since only the candidate is passed to it as an argument, otherwise the function itself would have to reference to the global value ivy-rich--display-transformers-list explicitly. So it can't perform the above shortening. I'm considering to pass the column properties (:width ... :face ...) as a second argument to the format function as well, but it seems a bit redundant...

@casouri
Copy link
Author

casouri commented Jul 9, 2018

How about making The file path format function always return the full path and let the transformer truncate the path on the fly? I'm sure the truncate function knows the required length at that time.

The idea is to separate responsibilities, format functions only provide raw information and the trimming and concatenation are done solely by the transformer.

@Yevgnen
Copy link
Owner

Yevgnen commented Jul 9, 2018

I agree with that.

Another issue is that, some ivy commands are from swiper, or counsel or even counsel-projectile or other xxx packages. If the minor mode set custom transformers before loading these commands, they may be overwritten when the corresponding package is loaded. Do you have any idea on this? Currently enabling ivy-rich-mode will force to load counsel.

@casouri
Copy link
Author

casouri commented Jul 10, 2018

I can think of two ways:

  1. advice ivy-set-display-transformer and filter the transformers. Most packages should follow the rule and use this function. However, if the package modifies ivy--display-transformers-list instead, the advice wouldn't work.

  2. In that case, we can add a function to the specific minor-mode's hook and override the change.

@Kaligule
Copy link

Kaligule commented Oct 6, 2021

Just wanted to mention that invy-filthy-rich is not maintained anymore. So this issue is now basically "try to get as many good ideas as possible from ivy-filthy-rich".

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

No branches or pull requests

3 participants