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 #338 fix: 🩹 #339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix #338 fix: 🩹 #339

wants to merge 1 commit into from

Conversation

fouss
Copy link
Contributor

@fouss fouss commented Jun 14, 2024

clean non ascii caracters returned by atOrUrl utils

@laem
Copy link
Owner

laem commented Jun 14, 2024

Je ne suis pas trop sûr de comprendre ta PR, tu peux expliquer un peu plus l'évolution du replace ?

@fouss
Copy link
Contributor Author

fouss commented Jun 14, 2024

yes en fait la regex trouvée ici : https://www.30secondsofcode.org/js/s/remove-non-ascii/ sert à supprimer les caractère qui "pourrissent" l'url, typiquement les espaces dans les liens comme pour les urls whatsapp qui donne ça actuellement :
https://wa.me/%E2%80%AA+33%C2%A06%C2%A052%C2%A065%C2%A075%C2%A059%E2%80%AC
Avec le replace de la regex ça donne ça : https://wa.me/+33652657559

En fait, dans ma précédente PR, je faisais bien le replace mais du mauvais coté du ternaire.

Au passage la regex n'est pas exactement la meme qu'avant par ce que la précédente (dont le résultat est identique) faisait pleurer mon eslint

@laem
Copy link
Owner

laem commented Jul 3, 2024

Mais du coup, je ne comprends toujours pas la PR :

  • est-ce que le fait de déplacer le replace asci en haut de la chaine de replace c'est impactant ?
  • est-ce que c'est grâce au changement de regexp que le bug liens cassés à cause des espaces #338 est corrigé ?

(désolé encore une fois j'aurais du réagir plus vite mais la conf SOTM m'a un peu mis dans le rouge 😅 )

@fouss
Copy link
Contributor Author

fouss commented Jul 3, 2024

Hello, pas de soucis ! Du coup j'essaie d'expliquer un peu :

  • le changement de la regex ne change rien, c'est une alternative qui à fit juste avec des linter un peu aggressif
  • du coup si tu regardes, avant j'avais le replace dans la partie "gauche" du ternaire donc ça n'impactait pas le return

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.

None yet

2 participants