Skip to content

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

Merged
koppor merged 12 commits into
JabRef:mainfrom
rishivardhanmm:fix-for-issue-12701
Mar 17, 2025
Merged

considering multiple spaces as separate user block by replacing it with " and "#12757
koppor merged 12 commits into
JabRef:mainfrom
rishivardhanmm:fix-for-issue-12701

Conversation

@rishivardhanmm

@rishivardhanmm rishivardhanmm commented Mar 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

koppor
koppor requested changes Mar 17, 2025
View reviewed changes

@koppor koppor left a comment

Copy link
Copy Markdown
Member

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:

Comment thread src/main/java/org/jabref/logic/importer/AuthorListParser.java Outdated
Comment thread src/test/java/org/jabref/logic/importer/AuthorListParserTest.java Outdated
koppor
koppor reviewed Mar 17, 2025
View reviewed changes
Comment thread src/test/java/org/jabref/logic/importer/AuthorListParserTest.java Outdated
@rishivardhanmm

rishivardhanmm commented Mar 17, 2025

Copy link
Copy Markdown
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
koppor reviewed Mar 17, 2025
View reviewed changes
Comment thread src/test/resources/org/jabref/logic/importer/fileformat/RisImporterTest1.ris Outdated
@koppor

koppor commented Mar 17, 2025

Copy link
Copy Markdown
Member

@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 (,)

koppor
koppor requested changes Mar 17, 2025
View reviewed changes
Comment thread CHANGELOG.md Outdated
Comment thread src/main/java/org/jabref/logic/importer/AuthorListParser.java Outdated
Comment thread src/main/java/org/jabref/logic/importer/AuthorListParser.java Outdated
Comment thread src/main/java/org/jabref/logic/importer/AuthorListParser.java Outdated
koppor
koppor reviewed Mar 17, 2025
View reviewed changes
Comment thread src/main/java/org/jabref/logic/importer/AuthorListParser.java Outdated
koppor
koppor reviewed Mar 17, 2025
View reviewed changes
Comment thread src/test/java/org/jabref/logic/importer/AuthorListParserTest.java
@trag-bot

trag-bot Bot commented Mar 17, 2025

Copy link
Copy Markdown

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

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

koppor commented Mar 17, 2025

Copy link
Copy Markdown
Member

@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
@rishivardhanmm

rishivardhanmm commented Mar 17, 2025

Copy link
Copy Markdown
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.

GuilhermeRibeiroPereira pushed a commit to GuilhermeRibeiroPereira/jabref that referenced this pull request Apr 1, 2025
@rishivardhanmm @koppor @GuilhermeRibeiroPereira
considering multiple spaces as separate user block by replacing it wi…
87a18a1 
…th " and " (JabRef#12757)

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

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

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

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

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

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

This reverts commit ca6e337

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

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

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

* Update CHANGELOG.md

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

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

---------

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@koppor koppor koppor approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

NormalizeNamesFormatter should treat spaces

2 participants

@rishivardhanmm @koppor