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

Strings reworked #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

comradekingu
Copy link
Contributor

@comradekingu comradekingu commented Dec 18, 2021

A lot of strings need to be put in quotes if that is to be the norm.

@kwart
Copy link
Member

kwart commented Dec 19, 2021

Thanks for the improvements, @comradekingu. I'll scan through them and review them.

@kwart kwart self-requested a review December 19, 2021 13:28
Copy link
Member

@kwart kwart left a comment

Choose a reason for hiding this comment

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

I went quickly through the messages and most of the changes improves the user experience, I think. Nevertheless, there are some issues that need to be resolved.

I usually commented only once for the issue type. So there can be other messages requiring the change (e.g. JSignPdf letter-casing, using non-ASCII characters in console. and hlp. messages).

@@ -4,7 +4,7 @@ certificationLevel.noChanges=No changes allowed
certificationLevel.notCertified=Not certified
console.certificateChainEmpty=No private key was found. Check the keystore settings (keystore type, filepath, password, key alias).
console.certificateExpired=Certificate {0} expired already.
console.certificateNotForSignature=Certificate {0} is not intended for digital signing.
console.certificateNotForSignature=The \"{0}\" certificate is not intended to be signed digitally.
Copy link
Member

Choose a reason for hiding this comment

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

I think this fix changes the meaning of the message.
IIUC the changed sentence, it sounds as we want to sign the certificate.
It's supposed to be the other way around. The certificate signs PDF. The X.509 certificates may contain a key-usage extension. It says if the certificate is intended for creating digital signatures.

Comment on lines +35 to +36
console.keys=Key aliases in the (JKS) keystore:
console.keystores=Available (JKS) keystore types:
Copy link
Member

Choose a reason for hiding this comment

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

The JKS shouldn't be mentioned here. The messages are used for non-JKS keystore types too (e.g. PKCS12, JCEKS, PKCS11)

error.vs.pageNotANumber=Page has to be an integer.
error.keystoreNull=The keystore was not loaded. Check if the keystore type, path and password are valid.
error.sslHandshakeException=SSL connection failed the certificate of the server is probably not known. You can use InstallCert Tool to import the certificate to Java Keystore.
error.vs.pageNotANumber=The page number has to whole number.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
error.vs.pageNotANumber=The page number has to whole number.
error.vs.pageNotANumber=The page number has to be the whole number.

@@ -13,7 +13,7 @@ console.createOutPdf=Creating output PDF file: {0}
console.createPdfReader=Opening input PDF file: {0}
console.createSignature=Creating signature
console.creatingTsaClient=Creating TSA client.
console.criticalExtensionNotSupported=Certificate {0} contains critical extension with OID "{1}" which is not recognized by JSignPdf - skipping.
console.criticalExtensionNotSupported=The {0} certificate contains a critical (OID "{1}") extension not recognized by JSignPDF — skipping.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.criticalExtensionNotSupported=The {0} certificate contains a critical (OID "{1}") extension not recognized by JSignPDF — skipping.
console.criticalExtensionNotSupported=The {0} certificate contains a critical extension (OID "{1}") not recognized by JSignPdf - skipping.
  • We should leave the name to be JSignPdf, it's used in the majority of places (web, jar-name, ...)
  • Also I would keep the console.* and hlp.* messages ASCII-based as some terminals (pointing on you, Windows cmd) could have issues with "advanced" character (, )

gui.certLevel.label=&Certification level
gui.certificateNotForEncryption.error=The file {0} either doesn't contain a X509 certificate or the encryption is not supported for the certificate.
gui.check.error.title=Check failed
gui.contact.label=Co&ntact (optional)
gui.encryptionCertFile.label=Certificate file for encryption
gui.fileNotExists.error={0} doesn''t exist or it is not readable.
gui.fileNotExists.error={0} doesn't exist or it is not readable.
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +195 to +197
hlp.ksFile=Sets (JKS) keystore file — as the value use the path on which is file with private key(s) located (.p12, .pfx, .jks, …). Some keystores haven't keys stored in a file (e.g. Windows keystore — WINDOWS-MY), then don't use this option.
hlp.ksPwd=Password to (JKS) keystore
hlp.ksType=Sets (JKS) keystore type (you can list possible values for this option -lkt argument)
Copy link
Member

Choose a reason for hiding this comment

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

Also here the JKS shouldn't be mentioned, as there are other types too.
And as help messages are printed to a terminal, I suggest avoiding the character.

hlp.loadProperties=Loads properties from a default file (created by GUI application).
hlp.loadPropertiesFile=Loads properties from the given file. The file can be create by copying the default property file .JSignPdf created by the GUI in the user home directory.
hlp.location=location of a signatue (e.g. Washington DC). Empty by default.
hlp.encryption=Encryption mode for the output PDF default value is NONE. Possible values are {0}. Use togethter with -upwd and -opwd in case of PASSWORD mode, and -ec in case of CERTIFICATE
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hlp.encryption=Encryption mode for the output PDF default value is NONE. Possible values are {0}. Use togethter with -upwd and -opwd in case of PASSWORD mode, and -ec in case of CERTIFICATE
hlp.encryption=Encryption mode for the output PDF. The default value is NONE. Possible values are {0}. Use together with -upwd and -opwd in case of PASSWORD mode, and -ec in case of CERTIFICATE

@kwart kwart force-pushed the master branch 6 times, most recently from f7efd80 to 9312988 Compare August 13, 2023 07:00
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.

2 participants