Skip to content

Implement function to prune string keeping HTML closing tags#1870

Merged
umputun merged 7 commits intoumputun:masterfrom
aliksend:aliksend/reduce-number-of-symbols-in-tg-message
Dec 4, 2025
Merged

Implement function to prune string keeping HTML closing tags#1870
umputun merged 7 commits intoumputun:masterfrom
aliksend:aliksend/reduce-number-of-symbols-in-tg-message

Conversation

@aliksend
Copy link
Copy Markdown
Contributor

@aliksend aliksend commented Dec 5, 2024

Fixes #1587

@aliksend aliksend requested a review from umputun as a code owner December 5, 2024 22:35
Copy link
Copy Markdown
Collaborator

@paskal paskal left a comment

Choose a reason for hiding this comment

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

Looks good! Could you please move the prune function to a different file? It's not telegram-specific and now increases complexity of telegram file without necessity.

@aliksend aliksend force-pushed the aliksend/reduce-number-of-symbols-in-tg-message branch from 0585517 to c16c2bf Compare December 17, 2024 04:59
@aliksend
Copy link
Copy Markdown
Contributor Author

Sure. Sorry for the long wait

Moved function to separated file and rebased my branch onto current master

paskal
paskal previously approved these changes Dec 17, 2024
Comment thread backend/app/notify/prune_html.go Outdated
@aliksend
Copy link
Copy Markdown
Contributor Author

I don't know why build fails on CI/CD. Locally I can do both go build -ldflags "-X main.revision=0.0.0 -s -w" from backend/_example/memory_store directory and docker buildx build --build-arg SKIP_BACKEND_TEST=true --build-arg SKIP_FRONTEND_TEST=true -f backend/_example/memory_store/Dockerfile . from root directory. Can you help me with it?

@umputun
Copy link
Copy Markdown
Owner

umputun commented Dec 18, 2024

I don't know why build fails on CI/CD. Locally I can do both go build -ldflags "-X main.revision=0.0.0 -s -w" from backend/_example/memory_store directory and docker buildx build --build-arg SKIP_BACKEND_TEST=true --build-arg SKIP_FRONTEND_TEST=true -f backend/_example/memory_store/Dockerfile . from root directory. Can you help me with it?

fixed, passed

paskal
paskal previously approved these changes Dec 18, 2024
@umputun
Copy link
Copy Markdown
Owner

umputun commented Dec 18, 2024

I have generated some unit tests, as the one it had didn't seem too convincing to me.

@umputun
Copy link
Copy Markdown
Owner

umputun commented Dec 18, 2024

pls note - the test is generated to match the code behavoir, however some of them look strange to me. for example this one

		{
 			name:      "text exceeding limit",
 			html:      "<p>Hello world, this is a long text</p>",
 			maxLength: 15,
 			expected:  "<p>...</p>",
 		},

I have not checked the code closely but the common sense expectation would be <p>Hello...</p> in this case.

@aliksend can you take a look pls and adjust tests/code if needed?

@aliksend
Copy link
Copy Markdown
Contributor Author

Current implementation will generate string "<p>Hello ...</p>" which length is 16, exceeding the limit. But it make sense to remove space in cases like this.
Also I'll fix another tests to make behavior better.

@paskal paskal force-pushed the aliksend/reduce-number-of-symbols-in-tg-message branch from 2e1dda5 to e2cf233 Compare March 12, 2025 23:03
@paskal paskal force-pushed the aliksend/reduce-number-of-symbols-in-tg-message branch from e2cf233 to 4b6e3ce Compare March 25, 2025 22:49
paskal
paskal previously approved these changes Mar 25, 2025
aliksend and others added 6 commits December 4, 2025 11:10
remove unneeded comment
Introduce comprehensive test cases for stringArr methods (Push, Pop, Unshift, Shift, String) to ensure correct behavior and state management. Additionally, add tests for HTML pruning functions (pruneHTML, pruneStringToWord) to validate handling of length constraints and formatting scenarios.
@paskal paskal force-pushed the aliksend/reduce-number-of-symbols-in-tg-message branch from 4b6e3ce to f5d591f Compare December 4, 2025 10:10
- Fix bug where HTML tags were counted toward the character limit
  instead of only visible text content
- Add pruning for parent comment text in Telegram notifications
- Simplify pruneStringToWord using strings.LastIndex
- Remove unused stringArr type and its tests
- Consolidate and simplify test cases
Copy link
Copy Markdown
Collaborator

@paskal paskal left a comment

Choose a reason for hiding this comment

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

  • Fixed pruneHTML to count only visible text (not HTML tags) toward the limit
  • Added pruning for parent comment text
  • Simplified code and tests (removed 306 lines net)

LGTM now.

@umputun umputun merged commit d01b738 into umputun:master Dec 4, 2025
3 checks passed
@paskal paskal added the backend label Mar 25, 2026
@paskal paskal added this to the v1.15.0 milestone Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce number of symbols in Telegram messages

3 participants