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

considering multiple spaces as separate user block by replacing it with " and " #12757

Merged
merged 12 commits into from
Mar 17, 2025

Conversation

rishivardhanmm
Copy link
Contributor

@rishivardhanmm rishivardhanmm commented Mar 17, 2025

Closes #12701

considering multiple spaces as separate user block by replacing it with " and "

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

image

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Nice that all tests go pass. I would have written a more closer check:

@rishivardhanmm
Copy link
Contributor Author

@koppor
image

This existing test case is failing, Looks like the current requirement is contradicting, I need your inputs, What if there is multiple spaces in between the first and second name?

@koppor
Copy link
Member

koppor commented Mar 17, 2025

@koppor image

This existing test case is failing, Looks like the current requirement is contradicting, I need your inputs, What if there is multiple spaces in between the first and second name?

Maybe, it the "guard" to the rewrite is to check for non existance of copmma (,)

Copy link

trag-bot bot commented Mar 17, 2025

@trag-bot didn't find any issues in the code! ✅✨

@koppor koppor added this pull request to the merge queue Mar 17, 2025
@koppor
Copy link
Member

koppor commented Mar 17, 2025

@rishivardhanmm Thank you for the quick replies. Now, this should be good to go.

Let's see if we encounter some erorr reports by our users... Getting heuristics right is always hard.

Merged via the queue into JabRef:main with commit f2ac9f0 Mar 17, 2025
31 checks passed
@rishivardhanmm
Copy link
Contributor Author

@rishivardhanmm Thank you for the quick replies. Now, this should be good to go.

Let's see if we encounter some erorr reports by our users... Getting heuristics right is always hard.

Thank you for your patience in review as well, I understand, Feel free to assign issues to me that comes back related to this.

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.

NormalizeNamesFormatter should treat spaces
2 participants