Add custom dir suffix colors

[JustinHallquist]

Description

This adds two things, one is a framework enhancement and the other is a feature request

Nested yaml structures

The framework enhancement is adding the ability to have nested yaml structure. Originally we convert all values in our yamls to symbols, but that was done at a flat depth -- keys were already symbols

This PR replaces that with a method that will convert all key/values to symbols at any depth so we can nest structure in our yamls

dir suffixes

This PR also adds the ability for users to specify custom dir suffixes

If a yaml has a structure that looks like

dir_suffixes:
  _suffix1: blue
  .suffix2: green

all directories ending in _suffix1 and .suffix2 will have the respective color

default

can remove

_venv:  royalblue

as a default if wanted

tests

are there any additional tests we want added for this change if we go with it?

documentation

what documentation, if any, would you like if we accept these changes?

• Relevant Issues : https://github.com/athityakumar/colorls/issues/206
• Relevant PRs : (none)
• Type of change :
  • [x] New feature
  • [ ] Bug fix for existing feature
  • [ ] Code quality improvement
  • [ ] Addition or Improvement of tests
  • [ ] Addition or Improvement of documentation

[harmnot]

should we put dir_suffixes: _suffix1: blue .suffix2: green on yams file?

to make other dirs coloured ???

[JustinHallquist]

@harmnot it can be added to either the user config or the yaml in the project:

      @filepath = File.join(File.dirname(__FILE__),"../yaml/#{filename}")
      @user_config_filepath = File.join(Dir.home, ".config/colorls/#{filename}")

I am fine removing the example here in lib/yaml/dark_colors.yaml and solely leaving it up to the user to implement it in their own config.

[securisec]

actually leaving the example is not a bad idea. as an example, because it supports . suffix, that line could be a different shade of blue to show hidden dirs as a lightly different color. and then from there user decides how else to scale it.

i tried cloning this repo, building with rake, and then getting a ghastly error message. Otherwise super excited for this to get merged.

Traceback (most recent call last):
	9: from /home/ubuntu/.linuxbrew/bin/colorls:23:in `<main>'
	8: from /home/ubuntu/.linuxbrew/bin/colorls:23:in `load'
	7: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/exe/colorls:5:in `<top (required)>'
	6: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/exe/colorls:5:in `new'
	5: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/lib/colorls/flags.rb:24:in `initialize'
	4: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/lib/colorls/flags.rb:168:in `parse_options'
	3: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/lib/colorls/flags.rb:176:in `set_color_opts'
	2: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/lib/colorls/yaml.rb:16:in `load'
	1: from /home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/lib/colorls/yaml.rb:16:in `map!'
/home/ubuntu/.linuxbrew/lib/ruby/gems/2.5.0/gems/colorls-1.1.1/lib/colorls/yaml.rb:16:in `block in load': undefined method `to_sym' for {"_venv"=>"royalblue"}:Hash (NoMethodError)
Did you mean?  to_s
               to_set

[JustinHallquist]

@securisec looks like you're on the wrong branch (line 16 is no longer map!) -- could you make sure you're on add-custom-dir-suffix-colors before running the rake install? I think that should clear up the issue

[securisec]

yes right, wrong branch. looks awesome!

[harmnot]

@JustinHallquist my dark_color.yaml

Main Colors

unrecognized_file: gold recognized_file: lavender dir: salmon

Access Modes

write: darkkhaki read: limegreen exec: red no_access: indianred

Age

day_old: mediumspringgreen hour_old: lime no_modifier: seagreen

File Size

file_large: orange file_medium: gold file_small: peachpuff

Random

report: white user: moccasin tree: cyan empty: yellow error: red normal: darkkhaki

Git

addition: chartreuse modification: darkkhaki deletion: darkred untracked: darkorange unchanged: forestgreen

dir_suffixes: pink _suffix1: blue

after save, it got error

[securisec]

@harmnot your dir_suffixes: pink does not seen right. Try removing pink from it.

[JustinHallquist]

yaml use indentation to denote structure.

for the case of these changes we wanted to ensure the added keys were namespaced properly or else it wouldnt be possible to determine the difference between a suffix and any other entry.

an example would be something such as: hello.tmp.rb and hello.tmp that yaml, without namespacing, would look like:

#flat hierarchy, cannot differentiate

.tmp: color_1 #the file type
.tmp: color_2 #the suffix

a hash representation would look something like this:

{
  .tmp: color_1,
  .tmp: color_2
}

to address that in yaml, we indent. for the case of this PR we use dir_suffixes as the namespace, indentation to denote it's a part of that name space, and key values to represent the suffixes and colors:

#namespace hierarchy, can find values correctly

.tmp: color_1 #the file type
dir_suffixes:
  .tmp: color_2 #the suffix

a hash representation would look something like this:

{
  .tmp: color_1,
  dir_suffixes: {
    .tmp: color_2
  }
}

for your case above:

addition: chartreuse
modification: darkkhaki
deletion: darkred
untracked: darkorange
unchanged: forestgreen

dir_suffixes:
  _suffix1: blue

please let me know if you have any more questions or if there is anything else regarding these changes you would like to go over.

[JustinHallquist]

Any status on the state of this PR?

[securisec]

@JustinHallquist any config or how to use changes on these new commits? i have been using your first version and it is very stable

[JustinHallquist]

@avdv hey so after pulling in master I noticed we added Forwardable in the last few weeks but never included it which was throwing an error when I tried to build.

I added that change in this PR but, assuming you can validate that bug, do you want me to move it to a separate fix?

[JustinHallquist]

@securisec just merged in master and dealt with some conflicts while waiting on this merge. No new changes in terms of features.

[avdv]

@avdv hey so after pulling in master I noticed we added Forwardable in the last few weeks but never included it which was throwing an error when I tried to build.

Yeah, I noticed that too. Bundler requires forwardable itself so tests were green.

I added that change in this PR but, assuming you can validate that bug, do you want me to move it to a separate fix?

Sure, that would be good. :+1:

I promise to give you a review next. :crossed_fingers:

[JustinHallquist]

@avdv forwardable addition is in this pr: https://github.com/athityakumar/colorls/pull/239

i'll deal with the merge conflict afterwards since it'll be trivial.

[JustinHallquist]

sorry for the delay, will get on that

[codecov-io]

Codecov Report

Merging #231 into master will increase coverage by 0.56%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
+ Coverage   84.52%   85.09%   +0.56%     
==========================================
  Files           7        7              
  Lines         420      436      +16     
==========================================
+ Hits          355      371      +16     
  Misses         65       65

Impacted Files	Coverage Δ
lib/colorls/core.rb	89.27% <100%> (+0.48%)	:arrow_up:
lib/colorls/yaml.rb	90% <100%> (+4.28%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5804f92...de87d0d. Read the comment docs.

[JustinHallquist]

@avdv bumpity bump anything I am missing to close this out?

[avdv]

@JustinHallquist I think we should add some documentation about this to the README into the "Custom configurations" section.

Also add a dir_suffix section to the light_colors.yaml file to keep it aligned to the dark_colorls.yaml file. Or should we remove it altogether? But an example of the usage would be good to have.

BTW, do you have some other examples for dir suffixes that could use a different color? Maybe there is something more common than *_venv folders... :thinking:

[JustinHallquist]

@avdv absolutely agree about the documentation -- will throw an update in the "Custom configurations" section.

I think removing it altogether makes sense as it should be on the consumer to determine which folders they want to stand out and the option will be documented for them to figure out how to do it. Otherwise it could be confusing as to why just that one folder in this pr has a different color.

I wouldn't be the best person to enquire around examples for this as I haven't a use case for dir suffixes. I'd personally prefer a prefix or regex matcher but that's me. The feature request was up and open source is powered by everyone chipping in 😄

However, everyone does handle their file systems differently so there are probably use cases that are eluding me such as cases, probably more common to devops, where folders are generated at fixed intervals by different services and tagging them and applying this could help with differentiation.

It's possible that this could end up being used as a fast implementation of single folder coloration. If I have a react app and I want __tests__ in each sub dir to pop, then I can just add the whole folder name to the yml and it'll match since the whole name is a valid suffix and apply the color.

I default to you on any concerns and necessities around the implementation of this, as well as requests for expanding scope or calling this an edge feature and abandoning. Though I would add that the addition to lib/colorls/yaml.rb extends the yamls to allow for nested attrs and that alone could be a benefit for future features.

Just let me know how you want to proceed.