Add custom dir suffix colors #231

JustinHallquist · 2018-10-23T19:06:28Z

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 : Feature request: Color certain directories a different color #206
Relevant PRs : (none)
Type of change :
- New feature
- Bug fix for existing feature
- Code quality improvement
- Addition or Improvement of tests
- Addition or Improvement of documentation

harmnot · 2018-10-24T15:21:28Z

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

to make other dirs coloured ???

JustinHallquist · 2018-10-25T04:00:36Z

@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 · 2018-10-25T04:59:23Z

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 · 2018-10-25T05:58:24Z

@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 · 2018-10-25T06:43:15Z

yes right, wrong branch. looks awesome!

harmnot · 2018-10-27T08:42:00Z

@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 · 2018-10-27T08:54:13Z

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

JustinHallquist · 2018-10-28T00:45:09Z

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 · 2018-11-01T02:37:22Z

Any status on the state of this PR?

securisec · 2018-11-19T18:48:22Z

@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 · 2018-11-19T18:48:29Z

@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 · 2018-11-19T18:50:19Z

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

avdv · 2018-11-20T07:37:29Z

@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. 👍

I promise to give you a review next. 🤞

JustinHallquist · 2018-11-20T16:45:33Z

@avdv forwardable addition is in this pr: #239

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

avdv

@JustinHallquist Please fix the rubocop offenses and see my comments.

avdv · 2018-12-29T12:52:04Z

lib/colorls/core.rb

+        name.end_with? suffix[0].to_s
+      end
+
+      suffix_arr ? @colors[:dir_suffixes][suffix_arr.first] : @colors[:dir]


The color is in suffix_arr[1], no need to fetch it again from the @colors hash.

thanks for pointing that out, updated

avdv · 2018-12-29T12:58:20Z

lib/colorls/core.rb

+      group = :folders
+      color = dir_color(content)
+      key =
+        if @folders.include?(key) && @folder_keys.include?(key)


@folder_keys is gone. And this is dead code, since key was never defined, so the condition will never be true.

JustinHallquist · 2019-01-15T05:39:03Z

sorry for the delay, will get on that

codecov-io · 2019-01-22T18:38:08Z

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%)`	⬆️
lib/colorls/yaml.rb	`90% <100%> (+4.28%)`	⬆️

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 · 2019-03-12T17:45:47Z

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

avdv · 2019-03-13T22:47:39Z

@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... 🤔

JustinHallquist · 2019-03-18T02:37:55Z

@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.

Justin Hallquist added 5 commits October 23, 2018 10:50

add custom dir suffix colors

d38f444

remove period dir delimiter until feedback

c68ce13

remove period dir delimiter until feedback

70c162d

generalize suffixes

2bf8633

fix lint issues

e2c54d7

Justin Hallquist added 4 commits November 19, 2018 09:46

fix merge conflicts

909ebb7

require forwardable

ad546f4

use folders var

81ddcbf

reduce complexity of options

0e5a9c3

Merge branch 'master' into add-custom-dir-suffix-colors

e457b20

ghost assigned avdv Dec 29, 2018

ghost added the review label Dec 29, 2018

avdv requested changes Dec 29, 2018

View reviewed changes

Justin Hallquist added 2 commits January 22, 2019 10:33

merge master and fix conflicts/dead code

7ea7775

address merge conflicts

de329ab

fix merge conflicts

de87d0d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom dir suffix colors #231

Add custom dir suffix colors #231

JustinHallquist commented Oct 23, 2018

harmnot commented Oct 24, 2018

JustinHallquist commented Oct 25, 2018

securisec commented Oct 25, 2018

JustinHallquist commented Oct 25, 2018 •

edited

securisec commented Oct 25, 2018

harmnot commented Oct 27, 2018

securisec commented Oct 27, 2018

JustinHallquist commented Oct 28, 2018

JustinHallquist commented Nov 1, 2018

securisec commented Nov 19, 2018

JustinHallquist commented Nov 19, 2018

JustinHallquist commented Nov 19, 2018

avdv commented Nov 20, 2018

JustinHallquist commented Nov 20, 2018 •

edited

avdv left a comment

avdv Dec 29, 2018

JustinHallquist Jan 22, 2019

avdv Dec 29, 2018

JustinHallquist Jan 22, 2019

JustinHallquist commented Jan 15, 2019

codecov-io commented Jan 22, 2019 •

edited

JustinHallquist commented Mar 12, 2019

avdv commented Mar 13, 2019

JustinHallquist commented Mar 18, 2019

Add custom dir suffix colors #231

Are you sure you want to change the base?

Add custom dir suffix colors #231

Conversation

JustinHallquist commented Oct 23, 2018

Description

Nested yaml structures

dir suffixes

default

tests

documentation

harmnot commented Oct 24, 2018

JustinHallquist commented Oct 25, 2018

securisec commented Oct 25, 2018

JustinHallquist commented Oct 25, 2018 • edited

securisec commented Oct 25, 2018

harmnot commented Oct 27, 2018

Main Colors

Access Modes

Age

File Size

Random

Git

securisec commented Oct 27, 2018

JustinHallquist commented Oct 28, 2018

JustinHallquist commented Nov 1, 2018

securisec commented Nov 19, 2018

JustinHallquist commented Nov 19, 2018

JustinHallquist commented Nov 19, 2018

avdv commented Nov 20, 2018

JustinHallquist commented Nov 20, 2018 • edited

avdv left a comment

Choose a reason for hiding this comment

avdv Dec 29, 2018

Choose a reason for hiding this comment

JustinHallquist Jan 22, 2019

Choose a reason for hiding this comment

avdv Dec 29, 2018

Choose a reason for hiding this comment

JustinHallquist Jan 22, 2019

Choose a reason for hiding this comment

JustinHallquist commented Jan 15, 2019

codecov-io commented Jan 22, 2019 • edited

Codecov Report

JustinHallquist commented Mar 12, 2019

avdv commented Mar 13, 2019

JustinHallquist commented Mar 18, 2019

JustinHallquist commented Oct 25, 2018 •

edited

JustinHallquist commented Nov 20, 2018 •

edited

codecov-io commented Jan 22, 2019 •

edited