-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
user-specific file directory should show user name #13380
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
jabgui/src/main/java/org/jabref/gui/libraryproperties/general/GeneralPropertiesView.java
Outdated
Show resolved
Hide resolved
userSpecificFileDirectoryTooltip.setText(userHost); | ||
laTexFileDirectoryTooltip.textProperty().bind( | ||
viewModel.laTexFileDirectoryProperty().map(path -> | ||
Localization.lang(path.isEmpty() ? "Directory for LaTeX files: (not set)" : "Directory for LaTeX files: %0", path) |
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.
This won't work this way, I think. You need to put the path.isEmpty check before and then have two separate Locallization.lang(...) calls
@@ -0,0 +1,23 @@ | |||
package org.jabref.logic.util; | |||
|
|||
public class UserAndHost { |
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.
This can be made a recod
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.
can i know what do you mean by record?
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.
Google for Java record
Or do Ctrl+Shift+F record in Intellij to see other record code.
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
# Conflicts: # .github/workflows/pr-comment.yml
.github/workflows/pr-comment.yml
Outdated
@@ -76,7 +76,7 @@ jobs: | |||
echo "workflow_run_id=${{ github.event.workflow_run.id }}" >> $GITHUB_OUTPUT | |||
fi | |||
|
|||
- name: Check if PR has 'dev: no-bot-comments' label |
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.
Why was there a need to touch the workflow file?
FieldPreferences getFieldPreferences(); | ||
|
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 think it was intentional to have each method separated by an empty line
@@ -0,0 +1,23 @@ | |||
package org.jabref.logic.util; | |||
|
|||
public class UserAndHost { |
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.
Google for Java record
Or do Ctrl+Shift+F record in Intellij to see other record code.
private final String host; | ||
|
||
public UserAndHost(String user, String host) { | ||
this.user = Objects.requireNonNull(user, "user must not be null"); |
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.
Use JSpecify annotations instead of requireNonNull
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
Signed-off-by: pranavdikshith <dikshithpranav@gmail.com>
@pranav0510s Please check your CHANGELOG.md modifications. You seem to have modified a section describing changes of a released version. See also the output of our tests. |
CHANGELOG.md
Outdated
@@ -107,6 +107,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv | |||
- We added path validation to file directories in library properties dialog. [#11840](https://github.com/JabRef/jabref/issues/11840) | |||
- We now support usage of custom CSL styles in the Open/LibreOffice integration. [#12337](https://github.com/JabRef/jabref/issues/12337) | |||
- We added support for citation-only CSL styles which don't specify bibliography formatting. [#12996](https://github.com/JabRef/jabref/pull/12996) | |||
- We added tooltips to the "User-specific file directory" and "LaTeX file directory" fields of the library properties window. [#12269](https://github.com/JabRef/jabref/issues/12269) |
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.
This must be put in the Unreleased section
@trag-bot didn't find any issues in the code! ✅✨ |
Your pull request needs to link an issue correctly. To ease organizational workflows, please link this pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue:
Examples
|
You modified Markdown ( You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "Markdown". |
@@ -108,6 +110,18 @@ public void initialize() { | |||
laTexSpecificFileDirSwitchTooltip.setText(isAbsolute ? switchToRelativeText : switchToAbsoluteText); | |||
}); | |||
|
|||
String userHost = preferences.getUsername() + "@" + preferences.getHostname(); |
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.
Should be part of record, maybe call method "getCanonicalForm()".
UserAndHost getUserAndHost(); | ||
|
||
/** Shortcut for getUserAndHost().getUser() */ | ||
default String getUsername() { | ||
return getUserAndHost().user(); | ||
} | ||
|
||
/** Shortcut for getUserAndHost().getHost() */ | ||
default String getHostname() { | ||
return getUserAndHost().host(); | ||
} | ||
|
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.
YAGNI. This clutters CliPreferences. Too much noise. getUserAndHost().host() is simple enough.
@@ -1456,7 +1457,7 @@ public InternalPreferences getInternalPreferences() { | |||
Version.parse(get(VERSION_IGNORED_UPDATE)), | |||
getBoolean(VERSION_CHECK_ENABLED), | |||
getPath(PREFS_EXPORT_PATH, getDefaultPath()), | |||
getUserAndHost(), | |||
getUserAndHost().toString(), |
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.
toString returns internal jdk string representation. this is outside of your control, use custom getCanonnicalForm() method instead.
laTexFileDirectoryTooltip.textProperty().bind( | ||
viewModel.laTexFileDirectoryProperty().map(path -> { | ||
if (path.isEmpty()) { | ||
return Localization.lang("Directory for LaTeX files: (not set)"); | ||
} else { | ||
return Localization.lang("Directory for LaTeX files: %0", path); | ||
} | ||
}) | ||
); |
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.
Closes Issue #12269
In this PR, I’ve refactored the CLI preferences to expose
getUsername()
andgetHostname()
default methods onCliPreferences
, updatedJabRefCliPreferences
to implement the publicgetUserAndHost()
interface method, and wired the General tab’s “User‐specific file directory” tooltip to displayuser@host
instead of the old String-array approach.Steps to test
username@hostname
.Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)