Skip to content

LScmd enhancement #256

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

Merged
merged 3 commits into from
Mar 29, 2019
Merged

LScmd enhancement #256

merged 3 commits into from
Mar 29, 2019

Conversation

hkchekc
Copy link
Member

@hkchekc hkchekc commented Mar 22, 2019

As the title suggested. refer to issue #152 and #190

I make all Modification related status green, Untracked red (more or less as in git) and removed files yellow.

Synced and other status where there is no content changes remain white.

I personally think it looks good. Only thing I am not sure is that if we should use different color for remote and local changes

@achilleas-k
Copy link
Member

Plain Printf() for colour doesn't work on Windows. You need to use Fprintf() with a colour writer.
For instance, the general purpose colour writer in our progress print functions:

fmt.Fprint(color.Output, newprint)

Not sure about the colour choices. I'd rather have synced files highlighted and untracked files not. I'd go for Green for Synced, Yellow for modified, Red for deleted, and White/uncoloured for untracked. This is different from what Git does, but Git uses Green for staged changes, which we consider some form of modified, but more importantly, the git status output doesn't show committed files which we do.

For the case switch where the colour is decided, the status variable is a FileStatus type. The Description() method is a convenience function for printing human readable text representing the status. It shouldn't be used to make any kind of programmatic decisions. For example, it could change at any time to make text clearer or the terms we use might change or it could be localised to different languages. FileStatus is an enum type and should be compared directly with the status constants (Synced, NoContent, etc) from the ginclient package.

@hkchekc
Copy link
Member Author

hkchekc commented Mar 27, 2019

Plain Printf() for colour doesn't work on Windows. You need to use Fprintf() with a colour writer.
For instance, the general purpose colour writer in our progress print functions:

gin-cli/gincmd/common.go

Line 235 in 5ee1573
fmt.Fprint(color.Output, newprint)

Not sure about the colour choices. I'd rather have synced files highlighted and untracked files not. I'd go for Green for Synced, Yellow for modified, Red for deleted, and White/uncoloured for untracked. This is different from what Git does, but Git uses Green for staged changes, which we consider some form of modified, but more importantly, the git status output doesn't show committed files which we do.

For the case switch where the colour is decided, the status variable is a FileStatus type. The Description() method is a convenience function for printing human readable text representing the status. It shouldn't be used to make any kind of programmatic decisions. For example, it could change at any time to make text clearer or the terms we use might change or it could be localised to different languages. FileStatus is an enum type and should be compared directly with the status constants (Synced, NoContent, etc) from the ginclient package.

Color changes and Fprintf included

Tell users what could they do with files under certain status
@hkchekc hkchekc changed the title Colorizing lscmd Outputs LScmd enhancement Mar 27, 2019
@hkchekc
Copy link
Member Author

hkchekc commented Mar 27, 2019

refer to also Issue #190. Add a few instructions to the gin ls output. The instructions suggest what can be the next step to deal with files under certain FileStatus

@achilleas-k
Copy link
Member

Looks good. Thanks!

@achilleas-k achilleas-k merged commit d786572 into G-Node:master Mar 29, 2019
@hkchekc hkchekc deleted the colorp branch March 29, 2019 12:03
@achilleas-k achilleas-k mentioned this pull request May 7, 2019
4 tasks
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