Skip to content

Fix column command calls #2

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

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

Fix column command calls #2

wants to merge 6 commits into from

Conversation

e-n-l
Copy link

@e-n-l e-n-l commented Apr 20, 2018

The current script returns the following message on execute:

column: option requires an argument -- 'n'
Try 'column --help' for more information.
column: option requires an argument -- 'n'
Try 'column --help' for more information.

This update supplies column with arguments sufficient to return the expected output

@ryanoasis
Copy link
Owner

@e-n-l Thanks for this 👍 but honestly I am confused. What is the difference as a result of this versus not even having the '-n' option? For me there is no visual difference. Can you show an example of the "-s"$symbol" -t " vs. no option?

Also to be perfectly honest I don't know why that is needed to delimit? From what I see in the code the $symbol is only used internally by the functions 🤔

@e-n-l
Copy link
Author

e-n-l commented Apr 22, 2018

TBH, I'm pretty new to bash... I was just trying to get it to behave based on what I'd read on the column man page, got something working and committed it without fully grokking what it was doing:

image

I've been trying to add color, based on better-ls, but it's screwed up the columns for some reason:

image

@ryanoasis
Copy link
Owner

Ah okay no worries.

Just wondering if removing the -n flag was enough or you had to add the flags you put in your pull request? I.e. did the coloring you are doing only working with the PR you submitted?

Would you mind sharing your code for color that you are doing or at least some of it? If you want that is 😃

@e-n-l
Copy link
Author

e-n-l commented Apr 23, 2018

So the top clip is what I'd submitted; seems to be working pretty well / consistently, but no colors.

The bottom is the WIP here, which I started doing when installing even-better-ls didn't work.

e-n-l and others added 2 commits April 25, 2018 02:26
Lambda character causes problems, for some reason... reverting to default src char. 
Remove dependency on Data::Dump
@e-n-l
Copy link
Author

e-n-l commented Apr 25, 2018

Okay, I really wanted this to work, so I just re-wrote the whole thing in Perl. I'm not sure if this even makes sense as a PR, or if I should make this a separate fork ala better-ls.

In any case, it works, it's pretty, and it's MUCH better organized for easy customization of icons and colors.

@ryanoasis
Copy link
Owner

Okay, I really wanted this to work, so I just re-wrote the whole thing in Perl

It looks great! I have honestly never used Perl but it wasn't too bad to figure out what you were doing and I understand the basic logic.

I'm not sure if this even makes sense as a PR, or if I should make this a separate fork ala better-ls.

So I was actually going back and forth on this. I first thought probably fork but I like the changes and the bash is still the basic 'glue' for all the logic. I think even calling it 'devicons-shell' even still follows though maybe there is an even better generic name.

Anyway, I'd like to merge this. I saw a problem with the alignment though:

selection_022

I could not quickly figure it out but somehow changing the echo to this: echo -e "$lines \n" | column -t | column worked for me:

selection_023

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.

2 participants