cafehaine / xontrib-xlsd

xontrib-xlsd is the next gen ls command for xonsh shell, inspired by lsd.
GNU General Public License v3.0
21 stars 4 forks source link

feat(ui): Add single whitespace between icon & name #34

Closed jd-solanki closed 2 years ago

jd-solanki commented 2 years ago

As a UI developer, I think there should a space between rendered icon and text so icon doesn't get stick to file/dir name.

NOTE: I just saw the code and made PR, if there's anything breaking please let me know.

cafehaine commented 2 years ago

Hi,

Thanks for your interest in xlsd.

I would prefer if this was configurable through an environment variable. For example:

XLSD_NAME_FORMAT="{icon} {name}"

This would allow for the feature you want, and give more customization options to the user.

jd-solanki commented 2 years ago

Hi,

I don't have much experience with "".format(). Can you please help me with how can I pass the {icon} {name} env var to name = "{}{}{{RESET}}".format(icon, name) because I don't think below is correct:

name = "$XLSD_NAME_FORMAT{{RESET}}".format(icon, name)

using eval is not recommended as well.

This is my first contribution to xonsh and I have to ask will I able to access the value of env var using below code snippet:

# Setting env
${...}.register('XLSD_NAME_FORMAT', type="str", default='{icon} {name}')

# Will this work 🤔 
print($XLSD_NAME_FORMAT)

Thanks.

cafehaine commented 2 years ago

I don't have much experience with "".format(). Can you please help me with how can I pass the {icon} {name} env var to name = "{}{}{{RESET}}".format(icon, name) because I don't think below is correct:

name = "$XLSD_NAME_FORMAT{{RESET}}".format(icon, name)

What you can do is the following:

name = $XLSD_NAME_FORMAT.format(icon=icon, name=name) + "{{RESET}}"

This doesn't use eval, and correctly formats the string.

This is my first contribution to xonsh and I have to ask will I able to access the value of env var using below code snippet:

# Setting env
${...}.register('XLSD_NAME_FORMAT', type="str", default='{icon} {name}')

# Will this work 🤔 
print($XLSD_NAME_FORMAT)

Yes exactly. For example, this is how I do it for ls -l columns: https://github.com/cafehaine/xontrib-xlsd/blob/43fa979f887c97bfd52ab3e7daf99e7b93614dd6/xontrib/xlsd.xsh#L126-L127 https://github.com/cafehaine/xontrib-xlsd/blob/43fa979f887c97bfd52ab3e7daf99e7b93614dd6/xontrib/xlsd.xsh#L575

jd-solanki commented 2 years ago

Hi,

Thanks for your guidance. I have updated the code, please review it and let me know if I can help you further :)

Regards.

cafehaine commented 2 years ago

Just tested your branch.

Aside from the note above, everything works as expected!

Thanks a lot for your contribution, will merge and make a new release once the note above is fixed :)

jd-solanki commented 2 years ago

Updated :)

Thanks for the suggestion.

cafehaine commented 2 years ago

Thanks again for your contribution, I will try to create a new release today.