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.

Sign up for GitHub

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

Jump to bottom

Corrected shortcut #6960

Merged
merged 10 commits into from
Oct 18, 2020
Merged

Corrected shortcut #6960

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5158e87
Corrected shortcut
Gena928 Sep 29, 2020
62a4fa8
6796 fixed
Gena928 Oct 13, 2020
692b398
Added new entry in JabRef_en.properties file
Gena928 Oct 14, 2020
87df8c7
Merge remote-tracking branch 'JabRefMaster/master' into 6867(2)
Gena928 Oct 14, 2020
bf6c0f7
Save before making changes
Gena928 Oct 14, 2020
3c3f918
Changed "getKeyCombination" method of KeyBindingRepository and all it…
Gena928 Oct 15, 2020
d480045
Corrected pull requests (comments from @calixtus)
Gena928 Oct 18, 2020
3d120ac
Corrected pull requests (comments from @calixtus)
Gena928 Oct 18, 2020
3c9981c
Merge branch 'master' into 6867(2)
calixtus Oct 18, 2020
2abea9e
Add whitespace in KeyBindingRepository
calixtus Oct 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where no longer a warning was displayed when inserting references into LibreOffice with an invalid "ReferenceParagraphFormat". [#6907](https://github.com/JabRef/jabref/pull/60907).
- We fixed an issue where a selected field was not removed after the first click in the custom entry types dialog [#6934](https://github.com/JabRef/jabref/issues/6934)
- We fixed an issue where a remove icon was shown for standard entry types in the custom entry types dialog [6906](https://github.com/JabRef/jabref/issues/6906)
- We fixed an issue, when pulling changes from shared database via shortcut caused creation a new new tech report [6867](https://github.com/JabRef/jabref/issues/6867)

### Removed

Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/jabref/gui/JabRefFrame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ private void initKeyBindings() {
case NEW_UNPUBLISHED:
new NewEntryAction(this, StandardEntryType.Unpublished, dialogService, prefs, stateManager).execute();
break;
case NEW_INPROCEEDINGS:
new NewEntryAction(this, StandardEntryType.InProceedings, dialogService, prefs, stateManager).execute();
break;
case PASTE:
if (OS.OS_X) { // Workaround for a jdk issue that executes paste twice when using cmd+v in a TextField
// Extra workaround for CodeArea, which does not inherit from TextInputControl
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jabref/gui/fieldeditors/TypeEditorViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

public class TypeEditorViewModel extends MapBasedEditorViewModel<String> {

private BiMap<String, String> itemMap = HashBiMap.create(8);
private BiMap<String, String> itemMap = HashBiMap.create(9);

public TypeEditorViewModel(Field field, SuggestionProvider<?> suggestionProvider, FieldCheckers fieldCheckers) {
super(field, suggestionProvider, fieldCheckers);
Expand All @@ -23,6 +23,7 @@ public TypeEditorViewModel(Field field, SuggestionProvider<?> suggestionProvider
itemMap.put("software", Localization.lang("Software"));
itemMap.put("datacd", Localization.lang("Data CD"));
itemMap.put("audiocd", Localization.lang("Audio CD"));
itemMap.put("inproceedings", Localization.lang("InProceedings"));
calixtus marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
15 changes: 8 additions & 7 deletions src/main/java/org/jabref/gui/keyboard/KeyBinding.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ public enum KeyBinding {
NEW_BOOK("New book", Localization.lang("New book"), "ctrl+shift+B", KeyBindingCategory.BIBTEX),
NEW_ENTRY("New entry", Localization.lang("New entry"), "ctrl+N", KeyBindingCategory.BIBTEX),
NEW_ENTRY_FROM_PLAIN_TEXT("New entry from plain text", Localization.lang("New entry from plain text"), "ctrl+shift+N", KeyBindingCategory.BIBTEX),
NEW_INBOOK("New inbook", Localization.lang("New inbook"), "ctrl+shift+I", KeyBindingCategory.BIBTEX),
NEW_MASTERSTHESIS("New mastersthesis", Localization.lang("New mastersthesis"), "ctrl+shift+M", KeyBindingCategory.BIBTEX),
NEW_PHDTHESIS("New phdthesis", Localization.lang("New phdthesis"), "ctrl+shift+T", KeyBindingCategory.BIBTEX),
NEW_PROCEEDINGS("New proceedings", Localization.lang("New proceedings"), "ctrl+shift+P", KeyBindingCategory.BIBTEX),
NEW_UNPUBLISHED("New unpublished", Localization.lang("New unpublished"), "ctrl+shift+U", KeyBindingCategory.BIBTEX),
NEW_TECHREPORT("New technical report", Localization.lang("New technical report"), "ctrl+shift+R", KeyBindingCategory.BIBTEX),
NEW_INBOOK("New inbook", Localization.lang("New inbook"), "", KeyBindingCategory.BIBTEX),
NEW_MASTERSTHESIS("New mastersthesis", Localization.lang("New mastersthesis"), "", KeyBindingCategory.BIBTEX),
NEW_PHDTHESIS("New phdthesis", Localization.lang("New phdthesis"), "", KeyBindingCategory.BIBTEX),
NEW_PROCEEDINGS("New proceedings", Localization.lang("New proceedings"), "", KeyBindingCategory.BIBTEX),
NEW_UNPUBLISHED("New unpublished", Localization.lang("New unpublished"), "", KeyBindingCategory.BIBTEX),
NEW_TECHREPORT("New technical report", Localization.lang("New technical report"), "alt+shift+R", KeyBindingCategory.BIBTEX),
calixtus marked this conversation as resolved.
Show resolved Hide resolved
NEW_INPROCEEDINGS("New inproceesings", Localization.lang("New inproceedings"), "alt+shift+C", KeyBindingCategory.BIBTEX),
NEXT_PREVIEW_LAYOUT("Next preview layout", Localization.lang("Next preview layout"), "F9", KeyBindingCategory.VIEW),
NEXT_LIBRARY("Next library", Localization.lang("Next library"), "ctrl+PAGE_DOWN", KeyBindingCategory.VIEW),
OPEN_CONSOLE("Open terminal here", Localization.lang("Open terminal here"), "ctrl+shift+L", KeyBindingCategory.TOOLS),
Expand All @@ -61,7 +62,7 @@ public enum KeyBinding {
OPEN_OPEN_OFFICE_LIBRE_OFFICE_CONNECTION("Open OpenOffice/LibreOffice connection", Localization.lang("Open OpenOffice/LibreOffice connection"), "alt+0", KeyBindingCategory.TOOLS),
OPEN_URL_OR_DOI("Open URL or DOI", Localization.lang("Open URL or DOI"), "F3", KeyBindingCategory.TOOLS),
PASTE("Paste", Localization.lang("Paste"), "ctrl+V", KeyBindingCategory.EDIT),
PULL_CHANGES_FROM_SHARED_DATABASE("Pull changes from shared database", Localization.lang("Pull changes from shared database"), "ctrl+shift+R", KeyBindingCategory.FILE),
PULL_CHANGES_FROM_SHARED_DATABASE("Pull changes from shared database", Localization.lang("Pull changes from shared database"), "alt+shift+D", KeyBindingCategory.FILE),
calixtus marked this conversation as resolved.
Show resolved Hide resolved
PREAMBLE_EDITOR_STORE_CHANGES("Preamble editor, store changes", Localization.lang("Preamble editor, store changes"), "alt+S", KeyBindingCategory.FILE),
PREVIOUS_PREVIEW_LAYOUT("Previous preview layout", Localization.lang("Previous preview layout"), "shift+F9", KeyBindingCategory.VIEW),
PREVIOUS_LIBRARY("Previous library", Localization.lang("Previous library"), "ctrl+PAGE_UP", KeyBindingCategory.VIEW),
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/jabref/gui/keyboard/KeyBindingRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ public int size() {

public Optional<KeyBinding> mapToKeyBinding(KeyEvent keyEvent) {
for (KeyBinding binding : KeyBinding.values()) {
if (checkKeyCombinationEquality(binding, keyEvent)) {
return Optional.of(binding);
if (!binding.getDefaultKeyBinding().isEmpty()){
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this works? I would gess getDefaultKeyBinding is always empty for NEW_INBOOK even though the user has overwritten it. On first glance, I would suggest to change the getKeyCombination method below to public Optional<KeyCombination> getKeyCombination(KeyBinding bindName) and return an empty optional if binding is empty.

Moreover, get should probably return en empty string instead of "Not associated" to be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works. If I change the next statement
from public KeyCombination getKeyCombination(KeyBinding bindName)
to Optional<KeyCombination> getKeyCombination(KeyBinding bindName)

I have to change other parts of code. For example line 141
from KeyCombination keyCombination = getKeyCombination(binding);
to KeyCombination keyCombination = getKeyCombination(binding).get();

Is this what you mean?

Copy link
Member

Choose a reason for hiding this comment

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

Okay, can you briefly explain why it works? I thought "getDefaultKeyBinding" then always returns an empty string, so this check is always false.

Yeah, there are more code changes required for my suggestion. I think it would provide a clean solution to handle the case that for a given keybinding there is not necessarily a keycombination. But I didn't had the time yet to check the code in detail. So if your solution indeed works, then it's fine with me as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started debugger and see that it always returns shortcut from KeyBinding.java file.

Screenshot 2020-10-14 at 12 56 37

Solution with Optional looks more clean. I just want to confirm that if we implement it, we also change other parts of code to getKeyCombination(binding).get().

Copy link
Member

Choose a reason for hiding this comment

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

In case this is done, the code fragment is getKeyCombination(binding).ifpresent(combination -> ...)

Copy link
Member

Choose a reason for hiding this comment

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

@Gena928 Exactly! So if you change the keybinding for "New inbook" to something, then getDefaultKeybinding should still return an empty string, so the code never goes into the if body in this case (although it should to match the new-user defined key).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tobiasdiez
I agree. If you change default shortcut, this code will not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try to fix today.

if (checkKeyCombinationEquality(binding, keyEvent)) {
return Optional.of(binding);
}
}
}
return Optional.empty();
Expand Down
4 changes: 3 additions & 1 deletion src/main/resources/l10n/JabRef_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2009,6 +2009,7 @@ Open\ files...=Open files...
Affected\ fields\:=Affected fields:
Show\ preview\ as\ a\ tab\ in\ entry\ editor=Show preview as a tab in entry editor
Font=Font
InProceedings=An article in a conference proceedings
Visual\ theme=Visual theme
Light\ theme=Light theme
Dark\ theme=Dark theme
Expand Down Expand Up @@ -2257,6 +2258,7 @@ Previous\ preview\ style=Previous preview style

(\ Note\:\ Press\ return\ to\ commit\ changes\ in\ the\ table\!\ )=( Note\: Press return to commit changes in the table\! )
Reset=Reset
New\ inproceedings=New inproceedings
Reset\ entry\ types\ and\ fields\ to\ defaults=Reset entry types and fields to defaults
This\ will\ reset\ all\ entry\ types\ to\ their\ default\ values\ and\ remove\ all\ custom\ entry\ types=This will reset all entry types to their default values and remove all custom entry types
Replace\ tabs\ with\ space=Replace tabs with space
Expand All @@ -2272,4 +2274,4 @@ This\ query\ uses\ unsupported\ syntax.=This query uses unsupported syntax.
Check\ Proxy\ Setting=Check Proxy Setting
Check\ connection=Check connection
Connection\ failed\!=Connection failed\!
Connection\ successful\!=Connection successful\!
Connection\ successful\!=Connection successful!