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

(exa PR) 1196: Update icons to nerd-font-3, replace few icons, handle compressed files separately #9

Closed
wants to merge 1 commit into from

Conversation

cafkafk
Copy link
Member

@cafkafk cafkafk commented Jul 29, 2023

@cafkafk cafkafk added the help wanted Extra attention is needed label Jul 29, 2023
@cafkafk cafkafk changed the title (exa PR) 1196 (exa PR) 1196: Update icons to nerd-font-3, replace few icons, handle compressed files separately Jul 29, 2023
@sbatial
Copy link
Collaborator

sbatial commented Jul 29, 2023

I've looked into this and I am all for it.

I would definitely consider it a breaking change so as always: There will be setups broken.
But that's a step worth taking considering we are taking about a font.

I was confused earlier because ogham/exa#1094 showed up as the Gentoo logo in my terminal.

@sbatial
Copy link
Collaborator

sbatial commented Jul 29, 2023

I'd just say we should decide on either blocking this by all other icon changes or vice versa because they have obvious overlap.

@cafkafk
Copy link
Member Author

cafkafk commented Jul 29, 2023

I'd just say we should decide on either blocking this by all other icon changes or vice versa because they have obvious overlap.

I don't use icons at all, so I'll deffer to your judgment on this.

I think we should mark all issues dealing with icons with a label first and then assess the whole situation, right now I don't really have an overview. Also, if we are lucky and the contributor that made the PR shows up I'd love to hear what they think about it.

@sbatial
Copy link
Collaborator

sbatial commented Jul 29, 2023

Ok, in that case: Let's block this one. It's easier to change/review one big one than every other small icon change in this case.

Marking all the icon-related ones sounds smart. 👍

@cafkafk
Copy link
Member Author

cafkafk commented Jul 31, 2023

This is affected by #28

@cafkafk cafkafk self-assigned this Jul 31, 2023
@cafkafk cafkafk added features › icon features › ui and removed help wanted Extra attention is needed labels Jul 31, 2023
@cafkafk
Copy link
Member Author

cafkafk commented Jul 31, 2023

I'm gonna try to have a crack at this but it's probably easier as a new pr, the merge would annoying. I'll add @Zer0-x as co-author on the commits.

Comment on lines -31 to +32
"Justfile", "Procfile", "Dockerfile", "Containerfile", "Vagrantfile", "Brewfile",
"Gemfile", "Pipfile", "build.sbt", "mix.exs", "bsconfig.json", "tsconfig.json",
"Justfile", "justfile", "Procfile", "Dockerfile", "Containerfile", "Vagrantfile",
"Brewfile", "Gemfile", "Pipfile", "build.sbt", "mix.exs", "bsconfig.json", "tsconfig.json",
Copy link
Member Author

Choose a reason for hiding this comment

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

done

"png", "jfi", "jfif", "jif", "jpe", "jpeg", "jpg", "gif", "bmp", "tiff",
"tif", "ppm", "pgm", "pbm", "pnm", "webp", "raw", "arw", "svg", "stl", "eps",
"dvi", "ps", "cbr", "jpf", "cbz", "xpm", "ico", "cr2", "orf", "nef",
"heif", "avif", "jxl", "j2k", "jp2", "j2c", "jpx", "pxm",
Copy link
Member Author

Choose a reason for hiding this comment

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

pxm is the only addition here

Copy link
Member Author

Choose a reason for hiding this comment

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

this is done

])
}

fn is_video(&self, file: &File<'_>) -> bool {
file.extension_is_one_of( &[
"avi", "flv", "m2v", "m4v", "mkv", "mov", "mp4", "mpeg",
"mpg", "ogm", "ogv", "vob", "wmv", "webm", "m2ts", "heic",
"mpg", "ogm", "ogv", "vob", "wmv", "webm", "m2ts", "heic", "video",
Copy link
Member Author

Choose a reason for hiding this comment

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

I've never seen this video format before, need example

"zip", "tar", "Z", "z", "gz", "bz2", "a", "ar", "7z",
"iso", "dmg", "tc", "rar", "par", "tgz", "xz", "txz",
"lz", "tlz", "lzma", "deb", "rpm", "zst", "lz4", "cpio",
"zip", "tar", "taz", "Z", "z", "gz", "bz", "bz2", "a", "ar",
Copy link
Member Author

Choose a reason for hiding this comment

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

taz and bz added here

Copy link
Member Author

Choose a reason for hiding this comment

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

done

"zip", "tar", "taz", "Z", "z", "gz", "bz", "bz2", "a", "ar",
"7z", "iso", "dmg", "tc", "rar", "par", "tgz", "xz", "txz",
"lz", "tlz", "lzma", "deb", "rpm", "zst", "lz4", "cpio", "lzh",
"lzo", "tbz", "tbz2", "tz", "tzo"
Copy link
Member Author

Choose a reason for hiding this comment

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

"lzh",
"lzo", "tbz", "tbz2", "tz", "tzo"

all seem new

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Comment on lines +140 to +142
else if self.is_compressed(file) {
Some(Icons::Compressed.value())
}
Copy link
Member Author

Choose a reason for hiding this comment

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

done

Self::Audio => '\u{f001}',
Self::Image => '\u{f1c5}',
Self::Video => '\u{f03d}',
Self::Audio => '\u{f001}', // 
Copy link
Member Author

Choose a reason for hiding this comment

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

this seems wrong

Comment on lines +25 to +28
Self::Audio => '\u{f001}', // 
Self::Image => '\u{f1c5}', // 
Self::Video => '\u{f03d}', // 
Self::Compressed => '\u{f410}', // 
Copy link
Member Author

Choose a reason for hiding this comment

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

done

Comment on lines -51 to 72
m.insert(".Trash", '\u{f1f8}'); //
// Icon for specific file name
m.insert(".atom", '\u{e764}'); // 
m.insert(".bashprofile", '\u{e615}'); // 
m.insert(".bashrc", '\u{f489}'); // 
m.insert(".git", '\u{f1d3}'); // 
m.insert(".gitattributes", '\u{f1d3}'); // 
m.insert(".gitconfig", '\u{f1d3}'); // 
m.insert(".github", '\u{f408}'); // 
m.insert(".gitignore", '\u{f1d3}'); // 
m.insert(".gitmodules", '\u{f1d3}'); // 
m.insert("gitignore_global", '\u{f1d3}'); // 
m.insert(".rvm", '\u{e21e}'); // 
m.insert(".vimrc", '\u{e62b}'); // 
m.insert(".vscode", '\u{e70c}'); // 
m.insert(".zshrc", '\u{f489}'); // 
m.insert("Cargo.lock", '\u{e7a8}'); // 
m.insert("bin", '\u{e5fc}'); // 
m.insert("config", '\u{e5fc}'); // 
m.insert("Cargo.lock", '\u{f1617}'); // 󱘗
m.insert("docker-compose.yml", '\u{f308}'); // 
m.insert("Dockerfile", '\u{f308}'); // 
m.insert("ds_store", '\u{f179}'); // 
m.insert("gitignore_global", '\u{f1d3}'); // 
m.insert("go.mod", '\u{e626}'); // 
m.insert("go.sum", '\u{e626}'); // 
m.insert("gradle", '\u{e256}'); // 
Copy link
Member Author

Choose a reason for hiding this comment

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

all from this point should be a separate PR imo

@cafkafk cafkafk deleted the pr-1196 branch September 12, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants