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

[WIP] Added better Ubuntu-GNOME ascii art #246

Merged
merged 1 commit into from May 14, 2016
Merged

[WIP] Added better Ubuntu-GNOME ascii art #246

merged 1 commit into from May 14, 2016

Conversation

@hashhar
Copy link
Contributor

@hashhar hashhar commented May 14, 2016

Everything's fine apart from the fact that the ASCII is getting too close to the output. I can see a variable called padding in the source but it doesn't seem like it's being used.

Also, I am trying to add an option for bold ASCII arts. I have a hack by changing: [0-7]) printf "%b%s" "\033[0m\033[3${1}m" to [0-7]) printf "%b%s" "\033[1m\033[3${1}m" on line 2642. But that makes everything bold and hence will not honor settings of a user for not using bold.

I wanted to ask how are you defining the colors for the ASCII?

2016-05-14_19-35-15_1366x768

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Thanks for the PR, I'm also getting the issue with the logo being too close.
I'll try and figure out why this is happening.

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

One thing I've noticed it that most of your logos are 52 characters wide (including the escape codes). Mine is 59. If I make it 52, it looks similar to what the default ones look like. I'll try and see if I can fit it into 52 chars.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

The padding is dynamic and should work with any width, how did you create this logo?

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

By hand almost. I had a simpler outline from somewhere I don't remember.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

This issue happens because the awk command I'm using to get the length of the longest line for padding outputs an incorrect value. What's weirder is that the issue still occurs when using
wc -L to get the length.

This is an issue with the ascii file itself, we just need to figure out what's causing it.

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

Hmmm, I think I've used some escapes like "`". I'll try using the command on smaller subsections of the ascii and tell you what I find.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Yup, I think it's that too. It works in this version without \ and '`'

'          ./o.
        .oooooooo
      .oooo```soooo
    .oooo`     `soooo
   .ooo`   .o.   `oooo.
   :ooo   :oooo.   `oooo.
    sooo    `ooooo    ooooo
     oooo    `soooo    `ooooo
      `soooo    `oooo    `soooo
./oo    `oooo    `oooo.   `ooo
`oooo.   `oooo.   `oooo.   ``
  `oooo.    oooo     ooo`
     `ooooo    ``    .oooo
       `soooo.     .oooo`
         `ooooooooooo`
            ``ooo``
'

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

Oh, then it's just a matter of removing them?

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Ok, this is actually a neofetch issue. Adding ascii_strip=${ascii_strip//\\\\/ } to line 2131 fixes the issue. I just tested with all 64 distro logos and it doesn't cause any issues.

The resoluting block should look like this:

    # Turn the file into a variable and strip escape codes.
    ascii_strip=$(<"$ascii")
    ascii_strip=${ascii_strip//\$\{??\}}
    ascii_strip=${ascii_strip//\\\\/ }
    ascii_strip=${ascii_strip//\\}

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

Okay. So do you want me to add it, or it would be added on master by you.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

I'll push it to master now.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Done.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Also in your ascii logo there's a 1 space gap on the left side, are you able to remove it for consistency with the other logos?

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

Sure. Done. It's working great on my end too.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Thanks, I also just pushed bold ascii art to master.

You can enable it by setting $ascii_bold to on or by using neofetch --ascii_bold on.

Loading

@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

Is this ready to be merged? LGTM

Loading

@hashhar
Copy link
Contributor Author

@hashhar hashhar commented May 14, 2016

LGTM. Thanks for the bold option.

Loading

@dylanaraps dylanaraps merged commit 46893fd into dylanaraps:master May 14, 2016
1 check passed
Loading
@dylanaraps
Copy link
Owner

@dylanaraps dylanaraps commented May 14, 2016

No problem. :)

Thanks for the PR.

Loading

@hashhar hashhar deleted the bold_ascii branch May 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants