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

v.util: improve code related to diff tool specified via environment, add check if the diff tool exists #21240

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Apr 9, 2024

The PR works on the util.diff module's code related to the VDIFF_TOOL environment variable.
It cleans up to help with maintainability and slightly extends by adding an error that should improve usability.

Regarding the changes:
At the beginning of the function we already checked if there is an env_difftool and return early. How env_difftool was used after this in the function, was quasi dead code.

if env_difftool != '' {
return '${env_difftool} ${env_diffopts}'
}

Then, instead of just returning the strings specified in the environment string it is also ensured here that specified command is executable. If not an error is returned to the user. E.g. when it isn't installed or if a typo or something.

@ttytm ttytm changed the title util: improve code related to diff to specified via environment, add check if command exists util: improve code related to diff tool specified via environment, add check if command exists Apr 9, 2024
Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Excellent work.

@spytheman spytheman changed the title util: improve code related to diff tool specified via environment, add check if command exists v.util: improve code related to diff tool specified via environment, add check if the diff tool exists Apr 11, 2024
@spytheman spytheman merged commit fa321ed into vlang:master Apr 11, 2024
56 checks passed
@ttytm ttytm deleted the util/env-diff-cmd branch April 11, 2024 14:29
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