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

Add more icons from eza #37

Closed
wants to merge 1 commit into from
Closed

Add more icons from eza #37

wants to merge 1 commit into from

Conversation

Freed-Wu
Copy link
Collaborator

@Freed-Wu Freed-Wu commented Dec 22, 2024

Add more icons from eza

Use scripts/generate-json.sh to generate:

  • assets/json/extension.json
  • assets/json/basename.json

https://github.com/orgs/eza-community/discussions/1270

Perhaps upstrem can provide json files directly.

Summary by CodeRabbit

  • New Features

    • Introduced new JSON files for file name and extension mappings, enhancing visual representation with corresponding icons.
    • Added a new shell script to automate JSON file generation from specified sources.
    • New Nix shell configuration file created for easier development environment setup.
  • Bug Fixes

    • Updated variable handling in Vim scripts to merge additional JSON data sources, improving configuration options.
  • Chores

    • Added new submodule configuration for better dependency management.

Copy link

coderabbitai bot commented Dec 22, 2024

Walkthrough

This pull request introduces a new submodule for the "eza" project and adds infrastructure for generating and managing JSON files containing icon mappings for file names and extensions. The changes include adding a new Git submodule, creating Perl and shell scripts to generate JSON files, introducing a Nix shell configuration, and updating Vim autoload scripts to incorporate additional icon mappings from the newly generated JSON files.

Changes

File Change Summary
.gitmodules Added new submodule for "eza" project with path and GitHub repository URL
eza New subproject commit added with identifier 500cd4faa849b8090a78c019f633cdeb92510b13
scripts/generate-basename.json.pl New Perl script to generate basename JSON with structured processing
scripts/generate-extension.json.pl New Perl script to generate extension JSON with line transformation
scripts/generate-json.sh New shell script to automate JSON file generation using Perl scripts
shell.nix New Nix shell configuration for development environment
assets/json/extra/basename.json New JSON file mapping file names to icons
assets/json/extra/extension.json New JSON file mapping file extensions to icons
autoload/nerdfont/path/basename.vim Updated to merge default and extra basename JSON configurations
autoload/nerdfont/path/extension.vim Updated to merge default and extra extension JSON configurations

Poem

🐰 Hop, hop, JSON in sight!
Submodules dancing with delight
Perl scripts weaving icons bright
Nerd Fonts now shine so right
A rabbit's code, pure and light! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
scripts/generate-extension.json.pl (1)

15-17: Add input validation for line transformations.

The current regex transformations assume specific input format. Consider adding validation:

+next if /^\s*$/;  # Skip empty lines
+next if /^\s*\/\//;  # Skip comment-only lines
+die "Malformed input line: $_" unless /=>/;  # Validate line format
 s|\s*=>.*//\s*|: "|;
 s|$|",|;
 print $_;
assets/json/basename.json (1)

2-62: Consider grouping related entries and adding missing icons.

The dotfile entries could benefit from better organization:

  1. Group shell-related files (.bash_*, .zsh_*)
  2. Group version control files (.git*)
  3. Add missing icons for config files
assets/json/extension.json (2)

88-88: Consider using Windows-specific icons for Windows-related extensions

The icons for Windows-specific extensions like exe and dll currently use generic icons. Consider using Windows-specific icons to better represent these file types.

-  "dll": "",
+  "dll": "",
-  "exe": "",
+  "exe": ""

Also applies to: 116-116


386-386: Standardize icons for subtitle-related extensions

The subtitle-related extensions (srt, ssa, sub) currently use "" which seems more related to Scala/SBT. Consider using a more appropriate media-related icon for consistency.

-  "srt": "󰨖",
+  "srt": "",
-  "ssa": "󰨖",
+  "ssa": "",
-  "sub": "󰨖",
+  "sub": ""

Also applies to: 388-388, 393-393

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6319ac3 and 67f0816.

📒 Files selected for processing (8)
  • .gitmodules (1 hunks)
  • assets/json/basename.json (1 hunks)
  • assets/json/extension.json (3 hunks)
  • eza (1 hunks)
  • scripts/generate-basename.json.pl (1 hunks)
  • scripts/generate-extension.json.pl (1 hunks)
  • scripts/generate-json.sh (1 hunks)
  • shell.nix (1 hunks)
✅ Files skipped from review due to trivial changes (4)
  • eza
  • .gitmodules
  • shell.nix
  • scripts/generate-json.sh
🔇 Additional comments (3)
assets/json/extension.json (3)

66-66: LGTM! Good use of download icons

The use of download icons ("󰇚") for temporary download files (*.part, *.crdownload) is appropriate and consistent.

Also applies to: 294-294


370-374: LGTM! Consistent use of hash icons

Good consistency in using the hash icon ("󰕥") across all SHA-related file extensions.


Line range hint 1-482: Verify JSON validity and key uniqueness

Let's ensure the JSON is valid and there are no duplicate keys.

✅ Verification successful

JSON structure verified successfully

The file is well-structured with valid JSON syntax and contains no duplicate keys.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify JSON validity and check for duplicate keys

# Check JSON validity
jq '.' assets/json/extension.json > /dev/null
if [ $? -eq 0 ]; then
    echo "JSON is valid"
else
    echo "Invalid JSON"
    exit 1
fi

# Check for duplicate keys
DUPLICATES=$(jq -r 'to_entries | group_by(.key) | map(select(length > 1)) | map(.[0].key)[]' assets/json/extension.json)
if [ -n "$DUPLICATES" ]; then
    echo "Found duplicate keys:"
    echo "$DUPLICATES"
    exit 1
else
    echo "No duplicate keys found"
fi

Length of output: 285

Comment on lines +1 to +4
#!/usr/bin/env -S perl -n
BEGIN {
print '{';
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling and input validation.

The script assumes well-formed input but should handle potential errors:

  • Invalid input format
  • Premature EOF
  • Missing constant definition
 #!/usr/bin/env -S perl -n
+use strict;
+use warnings;
+
 BEGIN {
     print '{';
+    $found_constant = 0;  # Track if we found the constant
 }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 1 to 18
#!/usr/bin/env -S perl -n
BEGIN {
print '{';
}

END {
print ' ".": ""' . "\n}";
}
if (/^const (?:DIRECTORY_ICONS|FILENAME_ICONS):/) {
$is_enabled = 1;
next;
}
$is_enabled = 0 if (/^};$/);
next unless $is_enabled;
s|\s*=>.*//\s*|: "|;
s|$|",|;
print $_;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider refactoring to reduce code duplication.

This script shares significant code with generate-extension.json.pl. Consider creating a shared module:

 #!/usr/bin/env -S perl -n
+use strict;
+use warnings;
+use lib 'scripts';  # Add scripts directory to module search path
+use IconProcessor;  # New shared module
 
-BEGIN {
-    print '{';
-}
-
-END {
-    print '    ".": ""' . "\n}";
-}
-if (/^const (?:DIRECTORY_ICONS|FILENAME_ICONS):/) {
-    $is_enabled = 1;
-    next;
-}
-$is_enabled = 0 if (/^};$/);
-next unless $is_enabled;
-s|\s*=>.*//\s*|: "|;
-s|$|",|;
-print $_;
+IconProcessor::process_input(
+    constants => ['DIRECTORY_ICONS', 'FILENAME_ICONS'],
+    output_file => 'assets/json/basename.json'
+);

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 63 to 202
"cron.d": "",
"cron.daily": "",
"cron.hourly": "",
"cron.minutely": "",
"cron.monthly": "",
"cron.weekly": "",
"crontab": "",
"crypttab": "",
"csh.cshrc": "󱆃",
"csh.login": "󱆃",
"csh.logout": "󱆃",
"docker-compose.yaml": "",
"docker-compose.yml": "",
"dune": "",
"dune-project": "",
"environment": "",
"etc": "",
"go.mod": "",
"go.sum": "",
"go.work": "",
"gradle": "",
"gradle.properties": "",
"gradlew": "",
"gradlew.bat": "",
"group": "",
"gruntfile.coffee": "",
"gruntfile.js": "",
"gruntfile.ls": "",
"gshadow": "",
"gulpfile.coffee": "",
"gulpfile.js": "",
"gulpfile.ls": "",
"hidden": "",
"license": "",
"heroku.yml": "",
"hi": "",
"hidden": "󱞞",
"home": "󱂵",
"hostname": "",
"id_dsa": "󰌆",
"id_ecdsa": "󰌆",
"id_ecdsa_sk": "󰌆",
"id_ed25519": "󰌆",
"id_ed25519_sk": "󰌆",
"id_rsa": "󰌆",
"include": "",
"inputrc": "",
"jsconfig.json": "",
"justfile": "",
"known_hosts": "󰣀",
"localized": "",
"makefile": "",
"mix.lock": "",
"npmignore": "",
"package.json": "",
"pkgbuild": "",
"procfile": "",
"rakefile": "",
"react.jsx": "",
"renovate.json": "󰉼",
"requirements.txt": "",
"localtime": "",
"makefile": "",
"node_modules": "",
"npm-shrinkwrap.json": "",
"npm_cache": "",
"npmrc": "",
"package-lock.json": "",
"package.json": "",
"pam.d": "󰢬",
"passwd": "",
"php.ini": "",
"platformio.ini": "",
"pom.xml": "",
"profile": "󱆃",
"pyproject.toml": "",
"release.toml": "",
"requirements.txt": "",
"robots.txt": "󰚩",
"rubydoc": "",
"tsconfig.json": "",
"yarn.lock": ""
"rvmrc": "",
"settings.gradle.kts": "",
"shadow": "",
"shells": "",
"ssh": "󰢬",
"sudoers": "",
"sudoers.d": "󰢬",
"timezone": "",
"tsconfig.json": "",
"webpack.config.js": "󰜫",
"xbps.d": "",
"xorg.conf.d": "",
"yarn.lock": "",
"zlogin": "󱆃",
"zlogout": "󱆃",
"zprofile": "󱆃",
"zshenv": "󱆃",
"zshrc": "󱆃"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Icon inconsistencies found and missing development files need to be added

  • .prettierrc is already handled in pattern.json with icon 󰏣, but missing in basename.json
  • .editorconfig is missing in both pattern.json and basename.json
  • License files (COPYING, LICENSE) and Makefile variants use empty icons "", which is consistent with the project's style
  • Add entries for:
    • .editorconfig: ""
    • .prettierrc: "󰏣" (matching pattern.json)
🔗 Analysis chain

Verify icon consistency across similar file types.

Some observations:

  1. License files (COPYING, LICENSE) use empty icons
  2. Build files (Makefile variants) could use consistent icons
  3. Some common development files are missing (e.g., .editorconfig, .prettierrc)

Let me check the pattern.json file to see how these development files are handled there.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for common development files that might be missing
rg -l '\.editorconfig|\.prettierrc' || echo "Files not found in codebase"

Length of output: 62


Script:

#!/bin/bash
# Check how development files are handled in pattern.json
cat assets/json/pattern.json | grep -A 5 -B 5 '\.editorconfig\|\.prettierrc'

Length of output: 466

@lambdalisue
Copy link
Owner

@Freed-Wu

I think the idea of using eza’s icons is excellent. However, incorporating this approach as it is in the current version poses challenges in terms of compatibility with previous versions. Since this plugin is licensed under MIT, you are free to modify it as long as the license requirements are met. How about forking it into a completely new plugin, such as vim-nerdfont-eza, and hosting it in the eza-community repository?

@Freed-Wu
Copy link
Collaborator Author

fork shouldn't be a good choice. It will bring confusion and split the power of contributors.

incorporating this approach as it is in the current version poses challenges in terms of compatibility with previous versions.

Currently, eza have more icons than this repo. we can utilize it to provide better user experience.
However, some icons of this project are missing in eza, which result in incompatibility with previous versions. IMO:

  1. PR to add them to eza.
  2. modify the script and merge the json with old data, not create a new json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
scripts/generate-basename.json.pl (2)

1-1: Limit usage of the -n option in the shebang.
Currently the script is invoked with “-n” which processes the input line-by-line, printing automatically. If you want fine-grained control, confirm whether you need it. Otherwise, you can remove or adapt it for clarity.


2-4: Encapsulate JSON opening brace.
Keeping the JSON printing logic within BEGIN/END blocks is fine but consider wrapping them in a function or module for testability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67f0816 and f4f0a5a.

📒 Files selected for processing (10)
  • .gitmodules (1 hunks)
  • assets/json/extra/basename.json (1 hunks)
  • assets/json/extra/extension.json (1 hunks)
  • autoload/nerdfont/path/basename.vim (1 hunks)
  • autoload/nerdfont/path/extension.vim (1 hunks)
  • eza (1 hunks)
  • scripts/generate-basename.json.pl (1 hunks)
  • scripts/generate-extension.json.pl (1 hunks)
  • scripts/generate-json.sh (1 hunks)
  • shell.nix (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • assets/json/extra/basename.json
🚧 Files skipped from review as they are similar to previous changes (5)
  • eza
  • .gitmodules
  • scripts/generate-extension.json.pl
  • scripts/generate-json.sh
  • shell.nix
🔇 Additional comments (3)
autoload/nerdfont/path/extension.vim (1)

2-2: Confirm consistency between 'basename' and 'extension' merges.
You're merging 'extension' with 'extra/basename'. Make sure the combination is intentional and does not cause key collisions or inconsistent icon associations.

✅ Verification successful

Let me gather information about the JSON files being merged and their potential key collisions.

This will help us:

  1. See the structure and content of the JSON files being merged
  2. Find any related files that might provide context about the intended merge behavior

Let me try a different approach to find and examine the JSON files being used.

This will help us:

  1. Understand where and how the JSON files are loaded
  2. Locate the actual JSON files being merged
  3. See the full context of the extension.vim file

Let's examine the contents of these JSON files to check for potential collisions.

This will help us:

  1. See the actual content of both JSON files being merged
  2. Identify any potential key collisions
  3. Understand the structure and purpose of each file

Merge is intentionally designed and safe to use

After examining both JSON files, the merge is safe and intentional because:

  1. extension.json contains file extension mappings (e.g., ".js", ".py")
  2. extra/basename.json contains specific filename mappings (e.g., ".gitignore", "package.json")
  3. The few overlapping keys (like ".bashrc", ".zshrc") in both files use the same icons, ensuring consistent behavior
  4. The merge enhances functionality by combining extension-based and filename-based icon mappings
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find and show contents of JSON files related to extension and basename
fd -e json -x cat {} \; | grep -A 20 -B 20 '"extension"\|"basename"'

# Additionally, let's look for any related configuration or test files
rg -l "extension.*basename|basename.*extension"

Length of output: 160


Script:

#!/bin/bash
# Look for the nerdfont#get_json function implementation to understand where these files are located
ast-grep --pattern 'function! nerdfont#get_json'

# Search for the actual JSON files in the repository
fd 'extension.json|basename.json' --type f

# Look at the implementation of the extension.vim file
cat autoload/nerdfont/path/extension.vim

Length of output: 894


Script:

#!/bin/bash
# Check contents of the relevant JSON files
echo "=== extension.json ==="
cat assets/json/extension.json
echo -e "\n=== extra/basename.json ==="
cat assets/json/extra/basename.json

Length of output: 8628

scripts/generate-basename.json.pl (2)

9-17: Refactor repeated logic with a shared module.
This script likely shares logic with the extension generator. You could put $is_enabled checking, line rewriting, and the final JSON formatting in a shared module.


6-8: Preserve consistent trailing commas.
Including a trailing comma for the last key-value pair in JSON might break strict JSON parsers. Confirm that downstream usage is flexible enough to handle this.

@@ -1,5 +1,5 @@
let g:nerdfont#path#basename#customs = get(g:, 'nerdfont#path#basename#customs', {})
let g:nerdfont#path#basename#defaults = nerdfont#get_json('basename')
let g:nerdfont#path#basename#defaults = extend(nerdfont#get_json('basename'), nerdfont#get_json('extra/basename'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

The concern is valid - no error handling for missing JSON files

The implementation of nerdfont#get_json doesn't include any error handling for missing JSON files. Both basename.json and extra/basename.json exist in the repository at assets/json/, but the function uses readfile() and json_decode() without any try-catch block, which would throw errors if the files are missing or corrupted.

  • autoload/nerdfont.vim: Add error handling around readfile() and json_decode() calls to return an empty dictionary when JSON files are missing.
🔗 Analysis chain

Ensure fallback behavior for extra JSON data.
While extending defaults with extra JSON data is beneficial, consider verifying if 'extra/basename' exists or is accessible. Otherwise, the plugin might fail if the JSON file is missing.


Let me verify the concern by checking the implementation of nerdfont#get_json function and how it handles missing JSON files.


I found the implementation of nerdfont#get_json, but let me check if there's any error handling for missing JSON files in the codebase.


Let me check the actual location of JSON files in the repository.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for the implementation of nerdfont#get_json function
ast-grep --pattern 'function! nerdfont#get_json($_) $_'

# Also search for any error handling related to JSON files
rg -g '*.vim' 'nerdfont#get_json' -A 5

Length of output: 2922


Script:

#!/bin/bash
# Check if there's any try-catch or error handling around nerdfont#get_json
rg -g '*.vim' 'try.*nerdfont#get_json|catch' -A 3

# Also check if the JSON files exist in the repository
fd -e json basename extra/basename

Length of output: 486


Script:

#!/bin/bash
# Find all JSON files in the repository
fd -e json

# Check the content of nerdfont#get_json function with more context
rg -g '*.vim' 'function! nerdfont#get_json' -B 5 -A 10

Length of output: 1154

Comment on lines +1 to +482
"sql": "",
"sqlite3": "",
"sr": "󱑻",
"srt": "󰨖",
"ss": "",
"ssa": "󰨖",
"stl": "󰆧",
"sty": "",
"styl": "",
"stylus": "",
"sub": "󰨖",
"sublime-build": "",
"sublime-keymap": "",
"sublime-menu": "",
"sublime-options": "",
"sublime-package": "",
"sublime-project": "",
"sublime-session": "",
"sublime-settings": "",
"sublime-snippet": "",
"sublime-theme": "",
"svelte": "",
"svg": "󰕙",
"swf": "",
"swift": "",
"t": "",
"tape": "󰨛",
"tar": "",
"taz": "",
"tbz": "",
"tbz2": "",
"tc": "",
"tex": "",
"tf": "󱁢",
"tfstate": "󱁢",
"tfvars": "󱁢",
"tgz": "",
"tif": "",
"tiff": "",
"tlz": "",
"tml": "",
"toml": "",
"torrent": "",
"tres": "",
"ts": "",
"tscn": "",
"tsv": "",
"tsx": "",
"ttc": "",
"ttf": "",
"twig": "",
"txt": "",
"txz": "",
"typ": "𝐭",
"tz": "",
"tzo": "",
"unity": "",
"unity3d": "",
"v": "",
"vdi": "",
"vhd": "",
"vhs": "󰨛",
"video": "",
"vim": "",
"vmdk": "",
"vob": "",
"vue": "󰡄",
"war": "",
"wav": "",
"webm": "",
"webmanifest": "",
"webp": "",
"whl": "",
"windows": "",
"wma": "",
"wmv": "",
"woff": "",
"woff2": "",
"wv": "",
"xcf": "",
"xci": "󰟡",
"xhtml": "",
"xlr": "",
"xls": "",
"xlsm": "",
"xlsx": "",
"xml": "󰗀",
"xpm": "",
"xul": "󰗀",
"xz": "",
"yaml": "",
"yml": "",
"z": "",
"z64": "󱎓",
"zig": "",
"zip": "",
"zsh": "",
"zsh-theme": "󱆃",
"zst": ""
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

After analyzing the output, I can now generate the final response as I've found the main extension mappings in assets/json/extension.json to compare against the extra entries.

Several icon collisions detected between built-in and extra extension mappings

The following extensions have conflicting icon definitions:

  • . (root): Both files define it with different icons ("" vs "")
  • android, apk, avi, avif, bash, bat, and many others have duplicate entries
  • Some extensions in the extra file have different icons than the built-in ones (e.g., cs, vue)

This could lead to inconsistent icon display depending on which file takes precedence during the merge. Consider:

  1. Removing duplicate entries from the extra extension file
  2. Ensuring intentional overwrites are documented
  3. Maintaining a single source of truth for extension mappings
🔗 Analysis chain

Check for collisions between built-in and extra entries.
Merging extension icons from multiple sources can lead to collisions or overwrites. Confirm that these entries don’t conflict with the main JSON.


Let me gather information about potential collisions between built-in and extra entries.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find all JSON files that might contain extension definitions
fd -e json -t f | while read -r file; do
  if [ "$file" != "assets/json/extra/extension.json" ]; then
    echo "=== $file ==="
    # Use jq to check if it's an object with string values (likely extension mappings)
    if jq -e 'select(type == "object" and all(.[]; type == "string"))' "$file" > /dev/null 2>&1; then
      cat "$file"
    fi
  fi
done

Length of output: 13870

Use scripts/generate-json.sh to generate:

- assets/json/extension.json
- assets/json/basename.json
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
assets/json/extra/extension.json (1)

8-8: Well-organized icon assignments with good consistency

The icon assignments show good consistency across related file types. For example:

  • Encryption-related files (.age, .gpg) use security-themed icons
  • Terraform-related files (.tf, .tfstate, .tfvars) use consistent Terraform icons

This makes the icon set intuitive and user-friendly.

Consider adding icons for these modern file types if they're supported by eza:

  • Container-related: .dockerfile, .containerfile
  • Package manager files: package.json, Cargo.toml
  • GitHub workflow files: .github/workflows/*.yml

Also applies to: 151-151, 416-418

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4f0a5a and 2637670.

📒 Files selected for processing (10)
  • .gitmodules (1 hunks)
  • assets/json/extra/basename.json (1 hunks)
  • assets/json/extra/extension.json (1 hunks)
  • autoload/nerdfont/path/basename.vim (1 hunks)
  • autoload/nerdfont/path/extension.vim (1 hunks)
  • eza (1 hunks)
  • scripts/generate-basename.json.pl (1 hunks)
  • scripts/generate-extension.json.pl (1 hunks)
  • scripts/generate-json.sh (1 hunks)
  • shell.nix (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
  • eza
  • shell.nix
  • assets/json/extra/basename.json
  • scripts/generate-basename.json.pl
  • scripts/generate-extension.json.pl
  • .gitmodules
  • autoload/nerdfont/path/extension.vim
  • scripts/generate-json.sh
  • autoload/nerdfont/path/basename.vim
🔇 Additional comments (2)
assets/json/extra/extension.json (2)

1-482: LGTM: Well-structured JSON format

The JSON structure is well-formed with consistent formatting and proper key-value pairs.


1-482: Verify consistency with eza's icon mappings

Since these icons are sourced from eza, let's verify they match eza's current icon set.

@@ -0,0 +1,482 @@
{
".": "",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Icon collisions with built-in mappings detected

As noted in previous reviews, there are icon collisions with the built-in mappings. For example:

  • Root directory "." has a different icon
  • "android" and "apk" have duplicate entries

This could lead to inconsistent icon display depending on which mapping takes precedence.

Consider one of these approaches:

  1. Remove duplicate entries and only keep icons that don't exist in the built-in mappings
  2. Document which mappings intentionally override built-in ones and why
  3. Implement a clear precedence rule in the code that loads these mappings

Also applies to: 14-14, 16-16

@lambdalisue
Copy link
Owner

It depends on the purpose of this PR.

If the purpose of this PR is simply to “add icons,” it would be welcomed. In that case, including a mechanism for synchronization wouldn’t be necessary.
However, if the purpose of this PR is to “provide the same icons as eza,” then forking would be a better approach since the core purpose of the plugin would essentially change.

Based on the changes in this PR, which include a synchronization script, I assumed it aligns with the latter purpose and hence suggested forking.

@Freed-Wu
Copy link
Collaborator Author

OK, I'll remove the mechanism for synchronization.

@Freed-Wu Freed-Wu closed this Dec 24, 2024
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