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

PocoDoc: search support #4494

Merged
merged 19 commits into from
Nov 11, 2024
Merged

PocoDoc: search support #4494

merged 19 commits into from
Nov 11, 2024

Conversation

vrabac6
Copy link
Contributor

@vrabac6 vrabac6 commented Mar 13, 2024

The PocoDoc did not support integration with FTS5 SQLite database for the purpose of inputting clean content into the database. To enhance its functionality, the following improvements have been implemented:

  1. Expanded the writeDescription method by adding an output stream containing clean content.
  2. Introduced a new method named writeSearchIndex , responsible for retrieving textual documentation along with links. This method is called within existing methods writeClass , writePackage , and writeNameSpace

@aleks-f aleks-f linked an issue Mar 27, 2024 that may be closed by this pull request
Copy link
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

  • Windows CI is failing; this needs update of vs90 project and regeneration of 160 and 170 projects
    D:\a\poco\poco\PocoDoc\src\DocWriter.cpp(29,10): error C1083: Cannot open include 
    file: 'Poco/Data/Session.h': No such file or directory 
    [D:\a\poco\poco\PocoDoc\PocoDoc_vs170.vcxproj]
    Error: Final attempt failed. Child_process exited with error code 1```
  • to prevent sanitizer CI failures, either pick these changes or simply merge devel branch
  • search support should be a runtime option
    • there should be a command line parameter added to PocoDoc to generate searchable SQLite DB
    • obviously, a precondition is SQLite built with FTS support
    • there should be a way to check at runtime if Data/SQLite supports FTS; only if it does can searchable content be produced (otherwise runtime error if searchable cmd line param was give)

@matejk matejk deleted the branch pocoproject:main April 15, 2024 11:20
@matejk matejk closed this Apr 15, 2024
@matejk matejk reopened this Apr 22, 2024
@matejk matejk changed the base branch from devel to main April 22, 2024 06:59
PocoDoc/src/PocoDoc.cpp Outdated Show resolved Hide resolved
@vrabac6 vrabac6 self-assigned this Oct 15, 2024
@vrabac6 vrabac6 requested a review from aleks-f October 15, 2024 16:27
Copy link
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

  • this PR is from devel branch, which was retired 6 months ago; pocoproject/main branch needs to be merged here before this PR is merged
  • FTS5 is available only on Android API Level > 24; see with @zaleksandar if this makes it worth keeping the FTS3/4 support alongside FTS5

Overall consolidation of the FTS capabilities is needed (this should probably be a separate issue):

  • there are discrepancies/inaccuracies in the build systems:
    • no need for =<path> here
    • this looks like mixing incompatible defines (probably the second one has no effect)
    • apparently, VS projects have FTS3 define hardcoded; on Windows FTS5 should be hardcoded in the projects (regenerated from SQLite.progen, not edited directly)
    • SQLite test has hardcoded FTS3
    • CMake build system should be adjusted to be in accordance with make build, coordinate with @matejk

@aleks-f aleks-f requested a review from matejk October 24, 2024 17:37
@vrabac6
Copy link
Contributor Author

vrabac6 commented Nov 1, 2024

  • FTS5 is available only on Android API Level > 24; see with @zaleksandar if this makes it worth keeping the FTS3/4 support alongside FTS5

According to the latest Android documentation, all new apps must target API level 34, and even older apps now target a minimum of API level 31. This makes it effectively impossible for apps to run on API levels lower than 24. Therefore, it should be safe and efficient to always enable FTS5 without needing to maintain support for FTS3/4.

PocoDoc/Makefile Show resolved Hide resolved
PocoDoc/Makefile Show resolved Hide resolved
@matejk
Copy link
Contributor

matejk commented Nov 4, 2024

PocoDoc.cmake.patch

Patch to correct CMake files for text search.

@vrabac6 vrabac6 requested a review from matejk November 4, 2024 11:20
@matejk matejk merged commit fe9c131 into pocoproject:main Nov 11, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

PocoDoc: search support
3 participants