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

fix: gave prompt when/if content was empty #686

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Shane-XB-Qian
Copy link
Contributor

    1. should be nice for user experience
    1. helpful to judge when/if pipeline

@Shane-XB-Qian Shane-XB-Qian requested a review from a team as a code owner February 13, 2025 10:51
@Shane-XB-Qian Shane-XB-Qian requested review from raphamorim and removed request for a team February 13, 2025 10:51
@Shane-XB-Qian Shane-XB-Qian force-pushed the fix_corner_case_empty_content branch from 0eb948e to 7748b07 Compare February 13, 2025 11:36
main.go Outdated
@@ -299,6 +299,10 @@ func executeCLI(cmd *cobra.Command, src *source, w io.Writer) error {
s = utils.WrapCodeBlock(string(b), ext)
}

if s == "" {
s = "[glow: empty content]"
Copy link
Member

Choose a reason for hiding this comment

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

prob @meowgorithm has a better idea of text placeholder for this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, empty content maybe confused, better to have one, but whatever the prompt to be.

@@ -22,4 +22,4 @@ jobs:
ruleguard:
uses: charmbracelet/meta/.github/workflows/ruleguard.yml@main
with:
go-version: stable
go-version: 1.23.6
Copy link
Member

Choose a reason for hiding this comment

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

is because stable isn't yet 1.23.6?

or to lock the lint to the project version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure, but err like ruleguard: internal error: package "os" without types was imported from "github.com/charmbracelet/glow/v2/utils"
// refer err at https://github.com/Shane-XB-Qian/glow/actions/runs/13306014402/job/37157054515

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after your team update mod yesterday, seems ruleguard still failed (if with 'stable'), but/vs here specific 1.23.6 work.

@Shane-XB-Qian
Copy link
Contributor Author

supposed i have found all places which should to add such prompt, pls check.

* 1. should be nice for user experience
* 2. helpful to judge when/if pipeline

Signed-off-by: shane.xb.qian <[email protected]>
Signed-off-by: shane.xb.qian <[email protected]>
@Shane-XB-Qian Shane-XB-Qian force-pushed the fix_corner_case_empty_content branch 2 times, most recently from 84c5108 to 94178d2 Compare February 14, 2025 13:54
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