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

Add a --version switch #743

Closed
tofi86 opened this issue Jan 12, 2017 · 10 comments
Closed

Add a --version switch #743

tofi86 opened this issue Jan 12, 2017 · 10 comments
Labels
status: has PR The issue is being processed in a pull request type: improvement The issue suggests an improvement of an existing feature
Milestone
4.1.0

Comments

@tofi86
Copy link
Collaborator

tofi86 commented Jan 12, 2017 •
edited
Loading

The epubcheck.jar should have a --version switch to display the version information only.
@tofi86 tofi86 added the type: improvement The issue suggests an improvement of an existing feature label Jan 12, 2017
@tofi86 tofi86 added this to the Next milestone Jan 12, 2017
@tofi86 tofi86 self-assigned this Feb 11, 2017
@tofi86 tofi86 added the status: waiting for feedback The development team needs feedback from the issue’s creator label Feb 17, 2017
@tofi86
Copy link
Collaborator Author

tofi86 commented Feb 17, 2017

hey @rdeltour, we already parse the argument string for a --version string in https://github.com/IDPF/epubcheck/blob/master/src/main/java/com/adobe/epubcheck/tool/EpubChecker.java#L541

Seems this was introduced as an alternative to the -v switch when using in conjunction with --mode / -m.

However, it's not documented anywhere. Not in commandline help --help nor in the wiki: https://github.com/IDPF/epubcheck/wiki/Running

What do you think, shall we just drop support for alternates -version and --version and make --version print the EpubCheck version number?

@tofi86 tofi86 removed the status: waiting for feedback The development team needs feedback from the issue’s creator label May 11, 2017
@tofi86 tofi86 removed their assignment May 11, 2017
@tofi86 tofi86 removed this from the 4.1.0 milestone May 11, 2017
@tofi86
Copy link
Collaborator Author

tofi86 commented May 11, 2017

Postponed to a later version, as this is causing conflicts with the current commandline arguments

@tofi86 tofi86 added the status: ready for implem The issue is ready to be implemented label Nov 27, 2017
@kalaspuffar kalaspuffar mentioned this issue Dec 14, 2017
Closed
@kamorrissey
Copy link
Contributor

I'd like to get my feet wet as a contributor, and this seemed like a modest item. If I understand correctly, the "ready for implementation" tag makes it suitable to work on. Is this right?

@kalaspuffar
Copy link
Contributor

Hi @kamorrissey

You are right that this issue is ready to be implemented. One thing I found in this project is that issues "ready for implementation" are usually well discussed and easy to get started with, as everything is in the open. This issue might have been left hanging because of the open-ended question in one of the later comments.

Glad to see your contribution and welcome.

Best regards
Daniel

@tofi86
Copy link
Collaborator Author

tofi86 commented Apr 18, 2018

Glad to see new volunteers, @kamorrissey! Welcome!

If we can agree on having two different meanings for the --version switch, this issue is ready to go!
I can live with it tbh.

@rdeltour
Copy link
Member

Glad to see new volunteers, @kamorrissey! Welcome!

+1, hi and welcome @kamorrissey 👋

If we can agree on having two different meanings for the --version switch, this issue is ready to go!
I can live with it tbh.

It seems the current --version (or -v) option is used to declare the EPUB spec version against which to check when validating single documents (with the --mode option), since in that case we can’t get the EPUB version from the OPF.

I agree that it can be pretty confusing with --version typically being used in a CLI to display the tool’s version.

We’d have a couple options:

  1. remove --version from its previous usage, and use it to display the tool’s version
  2. rename the -v|--version option to something else (e.g. -s|--spec-version, or -t|--target-version)
  3. keep the current use of the --version option, and make it display the tool’s version number only when the mode --mode is not specified

I don’t have a strong opinion; I don’t know if this option is heavily used out there. Option 3 is the less disruptive, but the most confusing.

@kamorrissey
Copy link
Contributor

I've been considering it. I'm in favor of option 1, since the --help message does not mention --version. I already have a proposed implementation that I'm going to submit in a pull request.

@rdeltour
Copy link
Member

I've been considering it. I'm in favor of option 1

Sounds good to me!

@kamorrissey
Copy link
Contributor

Created a pull request for review.

@tofi86 tofi86 added status: has PR The issue is being processed in a pull request and removed status: ready for implem The issue is ready to be implemented labels Apr 26, 2018
@tofi86 tofi86 added this to the 4.1.0 milestone Apr 26, 2018
@tofi86
Copy link
Collaborator Author

tofi86 commented Apr 26, 2018

PR #841

Awesome, thanks @kamorrissey 👏

We'll try to have a look asap!

@tofi86 tofi86 closed this as completed in e498c61 May 2, 2018
@tofi86 tofi86 assigned tofi86 and unassigned tofi86 May 2, 2018
fradelycodes added a commit to fradelycodes/epubcheck that referenced this issue Nov 11, 2024
@fradelycodes
Fix Readme Validations
2b210a1 
## Pull Request: Validation Corrections

### Description
This pull request includes several corrections to improve the validation process in EPUBCheck. The changes address various issues identified during the validation of EPUB files.

### Changes Made
- Corrected validation logic for EPUB version detection.
- Improved error handling for invalid `dc:identifier` UUIDs.
- Updated the validation rules for `--version` and `-version` command line options.
- Enhanced file extension checks to include non-`.epub` files.
- Adjusted the reporting of `file://` URLs to `INFO` level.

### Related Issues
- Closes w3c#650
- Closes w3c#498
- Closes w3c#853
- Closes w3c#743
- Closes w3c#490
- Closes w3c#289

### Testing
- Added unit tests to cover the new validation rules.
- Verified that the changes do not introduce any regressions.

### Additional Notes
- Please review the changes and provide feedback.
- Contributions from the community are welcome to further improve the validation process.

Thank you for your support and contributions to EPUBCheck!

---

**Contributors:**
- @fradelycodes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: has PR The issue is being processed in a pull request type: improvement The issue suggests an improvement of an existing feature
Projects
None yet
Milestone
4.1.0
Development

No branches or pull requests
4 participants
@tofi86 @rdeltour @kalaspuffar @kamorrissey