Bugzilla – Full Text Bug Listing
| Summary: | Add support for individual element validation to Autoconf parser | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Sami Wagiaalla <swagiaal> | ||||||||||
| Component: | Autotools | Assignee: | Jeff Johnston <jjohnstn> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | ||||||||||||
| Version: | unspecified | ||||||||||||
| Target Milestone: | --- | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Linux | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
Thanks for the patch, Sami! I like where this is going. There are some unnecessary blank line additions and some minor indentation changes (perhaps they're corrections?) but otherwise it looks pretty good. I'll let Jeff do a full review. Comment on attachment 197417 [details]
Add support for individual element validation.
1. AcInitElement should base it's logic on the current Autoconf Editor version in project properties. 2.13 means one argument which is a file or directory that exists in the top-level directory. 2.59 and higher means first argument is project name, 2nd argument is version number and there may be two additional optional arguments (e.g. 3 or 4 arguments total).
2. Error names should be less generic (badProjectName and badVersionNumber for example).
3. Bad version number message should have "be a version number" not "must the version number".
4. "AC_INIT" string being compared to should be denoted with //$NON-NLS-1$
Other than that looks good and you provided a ChangeLog entry.
Created attachment 197510 [details]
Add support for individual element validation.
- Made macro validation aware of current version
- Removed incorrect validation of first argument (no restrictions)
- Removed misc new lines changes.
- Improved error message names.
(In reply to comment #1) > Thanks for the patch, Sami! I like where this is going. There are some > unnecessary blank line additions and some minor indentation changes (perhaps > they're corrections?) Most were just artefacts, AutoconfMacro was intentional as indicated in the Changelog. All clean up now. Created attachment 197543 [details]
Add support for individual element validation.
- Use plugin default when version property is not set
Comment on attachment 197543 [details]
Add support for individual element validation.
Looks good. Only remaining comment I would have would be to add a simple Comparator class for the version strings. That way, the AC_INIT macro can check for >=2.59 rather than hard-code the two latest current choices. This means no updating required if a new version gets added. The Comparator could simply compare the first numeral and then the second following the decimal-point (e.g. "2.61" > "2.59", "2.61" < "2.129").
Created attachment 197786 [details]
Add support for individual element validation.
- Added requested VersionComparator class to be used for version comparison
(In reply to comment #7) > Created attachment 197786 [details] > Add support for individual element validation. > > - Added requested VersionComparator class to be used for version comparison Looks good. Thanks. Code checked in to repository. Thanks. |
Created attachment 197417 [details] Add support for individual element validation. The autoconf parser does not allow individual elements to validate their arguments. I am attaching a patch which adds that support and uses it in validating AC_INIT as an example use case.