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

Working with options / flags #4

Closed
athityakumar opened this issue Jun 30, 2017 · 10 comments
Closed

Working with options / flags #4

athityakumar opened this issue Jun 30, 2017 · 10 comments

Comments

@athityakumar
Copy link
Owner

ls -l works, but something like lc -l doesn't. Having this feature to highlight file formats in strings would be a great feature.

@athityakumar
Copy link
Owner Author

@kaustubhhiware - Can you confirm whether this is fixed on current master by pulling it? (as of commit 23d95fc)

That is, usage like lc -al . ../ ~/

@kaustubhhiware
Copy link
Contributor

kaustubhhiware commented Jun 30, 2017

Umm, there is good news and bad news.
Good news:

  • Flags work

Bad news:

  • I'm getting a warning previously unreceived: colorls/core.rb:25: warning: Insecure world writable dir ~/.config in PATH, mode 040777
  • Why are all the listings only one per row ?

@athityakumar
Copy link
Owner Author

athityakumar commented Jul 1, 2017

Understood. Previously, lc was only to work without options and hence it had custom indents and stuff. But now that I want lc to colorize any output of ls, it has to mimic it.

Currently, it splits by newline and then space to get all words. If word is file / directory, colorize. Else, just print.

This is the reason behind the issue #5 too. It somehow now has to be made to work without splitting with \n and space - and rather just do string manipulation on output of ls.

@athityakumar
Copy link
Owner Author

@kaustubhhiware - Just had a look at this link and seems like it was a bad idea to try adding all ls flag features to lc. I think it'd be better to keep lc simple with just dirs support, rather than printing 1 file / line just for supporting flags. WDYT?

@kaustubhhiware
Copy link
Contributor

Hmm, maybe just stick with whatever best you do. According to me, only two flags could be useful to someone:

I don't think all flags need to be covered, for instance, if I were interested in more detailed information, I would just do ls -l, and not lc -l.

@athityakumar
Copy link
Owner Author

  1. lc is automatically returning all dotfiles by default.
  2. Yep, added this. Check the latest master (1fbd3a7).

@kaustubhhiware
Copy link
Contributor

kaustubhhiware commented Jul 1, 2017

Can confirm this as per commit 405b3f0. Can close this issue now.

Although I would suggest to add some error handling. If I were to execute lc -a, or something incorrect like that, unnecessary flags can be ignored, and some warning message can be printed. As far as I remember, this is possible in shell, not so sure about ruby.

Current response to incorrect command:

$ lc -a                                                                 123ms  Saturday 01 July 2017 07:53:57 PM IST
colorls/colorls.rb:10:in `open': No such file or directory @ dir_initialize - -a (Errno::ENOENT)
	from colorls/colorls.rb:10:in `entries'
	from colorls/colorls.rb:10:in `initialize'
	from colorls/colorls.rb:121:in `new'
	from colorls/colorls.rb:121:in `block in <main>'
	from colorls/colorls.rb:121:in `each'
	from colorls/colorls.rb:121:in `<main>'

@athityakumar
Copy link
Owner Author

Thanks! Closing this issue now. 😄

@kaustubhhiware
Copy link
Contributor

kaustubhhiware commented Jul 1, 2017

@athityakumar I think this needs to be known.
The number of content names in different locations varies. Relevant comment above.

This seems intentional, since the number of listings per row seems to depend on the longest name in that folder. This is in no way an issue, but might as well check.

@athityakumar
Copy link
Owner Author

@kaustubhhiware - Currently, it doesn't depend on the longest name in the folder, but the longest in each column being shown. But sure, error handling is required. Or rather, proper logic to rule out the incorrect flags (so, except --report for now).

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

2 participants