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

Added support for UTF-8 clipboard based on TigerVNC/TightVNC extension #27

Closed
wants to merge 2 commits into from

Conversation

egorksv
Copy link

@egorksv egorksv commented May 2, 2022

No description provided.

Copy link
Owner

@shinyhut shinyhut left a comment

Choose a reason for hiding this comment

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

Hi @egorksv thanks for this, it looks good in principle. I've had a quick run through and I think this PR includes a lot of changes that are specific to your fork and build environment (POM changes, default passwords etc) - I think you'll need to create a branch in your fork without these changes and open a PR from there. If you get that sorted out I'll have a closer look at the logic and hopefully merge.

@@ -6,7 +6,7 @@
<groupId>com.shinyhut</groupId>
<artifactId>vernacular</artifactId>
<packaging>jar</packaging>
<version>1.15-SNAPSHOT</version>
<version>1.15.1</version>
Copy link
Owner

Choose a reason for hiding this comment

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

Undo this

@@ -96,6 +96,7 @@
</manifest>
</archive>
</configuration>

Copy link
Owner

Choose a reason for hiding this comment

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

Remove the blank line

<id>ossrh</id>
<url>https://s01.oss.sonatype.org/service/local/staging/deploy/maven2/</url>
<id>maven.nuvalence.repo</id>
<url>s3://testup-build-artifacts/m2</url>
Copy link
Owner

Choose a reason for hiding this comment

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

Undo this

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>11</source>
Copy link
Owner

Choose a reason for hiding this comment

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

Undo this - I want to keep this targeting Java 8 for the time being

@@ -299,9 +300,9 @@ private void addMenu() {

private void showConnectDialog() {
JPanel connectDialog = new JPanel();
JTextField hostField = new JTextField(20);
JTextField hostField = new JTextField("localhost", 20);
Copy link
Owner

Choose a reason for hiding this comment

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

Leave this empty by default

hostField.addAncestorListener(focusRequester);
JTextField portField = new JTextField("5900");
JTextField portField = new JTextField("5901");
Copy link
Owner

Choose a reason for hiding this comment

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

Leave this defaulted to 5900

@@ -342,9 +343,9 @@ private String showUsernameDialog() {
}

private String showPasswordDialog() {
String password = "";
String password = "headless";
Copy link
Owner

Choose a reason for hiding this comment

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

Leave this empty by default

JPanel passwordDialog = new JPanel();
JPasswordField passwordField = new JPasswordField(20);
JPasswordField passwordField = new JPasswordField("headless",20);
Copy link
Owner

Choose a reason for hiding this comment

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

And this

* @return
* @throws IOException
*/
private static ServerCutText readExtendedClipboard(DataInputStream in, int textLength, ClipboardEventHandler handler) throws IOException {
Copy link
Owner

Choose a reason for hiding this comment

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

This should really have a unit test

<extensions>
<extension>
<groupId>com.github.seahen</groupId>
<artifactId>maven-s3-wagon</artifactId>
Copy link
Owner

Choose a reason for hiding this comment

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

Remove this

Copy link
Author

Choose a reason for hiding this comment

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

Hi Paul

We've encountered a few issues with this feature - just wanted to ensure they're all ironed out before we proceed with the pull request.

Cheers
Egor

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.

None yet

2 participants