-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fixed switching off autocomplete on specific fields #7496
Conversation
…ose to ignore autocompletion for that particular field
src/test/java/org/jabref/gui/autocompleter/FieldValueSuggestionProviderTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Christoph <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @BJaroszkowski . Especially thanks for providing also a test for your change. Thanks for your contribution so far.
One small suggestion and a bigger request: Please do not move stuff around, if they are not directly related to changes. This easily leads merge errors, if others are working on the class (in this case LibraryTab) too, makes it more complicated to review and there is also in many classes some kind of order we try to keep with the definitions to make it easier to read and understand. So please undo the unnecessary changes in LibraryTab.java
BibEntry entry = new BibEntry(); | ||
entry.setField(StandardField.TITLE, "testValue"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new BibEntry().withField()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to admit I was not aware of existence of this method, this would certainly be an improvement. The thing is that all the tests are written in the manner I did it. It would probably be a good idea to change them all at once for the sake of consistency. This however sounds like something for another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I think the changes in the LibraryTab come from (automatically) running the code formatter. Should be fine.
Yes, this is exactly the case. Should I roll back the changes anyway? |
Nope, I checked the currently open PRs in the issue tracker and think there is none, that depends on LibraryTab, so I guess we can merge. JabRefFrame and LibraryTab need some refactoring anyway. |
This fixes issue described in #7320. Even though user could choose fields on which autocomplete should not work it had no actual effect.
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)