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

Improve bookmarks html reader #1986

Merged
merged 4 commits into from
Dec 21, 2023
Merged

Improve bookmarks html reader #1986

merged 4 commits into from
Dec 21, 2023

Conversation

mallexxx
Copy link
Collaborator

@mallexxx mallexxx commented Dec 19, 2023

Task/Issue URL: https://app.asana.com/0/0/1206196320966558/f

Description:

Steps to test this PR:

  1. Validate html import from browsers (using provided test html files)

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@mallexxx mallexxx requested a review from ayoy December 19, 2023 09:58
Copy link
Contributor

github-actions bot commented Dec 19, 2023

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 470bc4d

@ayoy ayoy self-assigned this Dec 20, 2023
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

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

This works great @mallexxx! I left just a tiny comment, but all looks good and works perfectly fine. I tested also with some of my test HTML files, with no issues whatsoever.

One more thing – there's a readme file outlining bookmarks HTML reader working principle, nicely hidden in the repository: https://github.com/duckduckgo/macos-browser/tree/main/DuckDuckGo/DataImport/Bookmarks/HTML. It's definitely outdated with your changes, so we can either delete it or try to update it (I created it while adding HTML import hoping that it would be helpful to others, you of course seem to have dealt with it without extra resources, but maybe it's worth to keep these diagrams up to date for posterity (and maybe move them elsewhere).

Thanks again for the patch!

@@ -12953,6 +12976,14 @@
version = 2.5.1;
};
};
B65CD8C92B316DF100A595BB /* XCRemoteSwiftPackageReference "swift-snapshot-testing" */ = {
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/pointfreeco/swift-snapshot-testing";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding this, it's a great library!

@mallexxx
Copy link
Collaborator Author

Thanks, @ayoy! I‘ve updated the readme file and added it to the project so it‘s visible

@mallexxx mallexxx merged commit 6389927 into main Dec 21, 2023
14 checks passed
@mallexxx mallexxx deleted the alex/improve-html-import branch December 21, 2023 09:23
samsymons added a commit that referenced this pull request Dec 21, 2023
# By Dominik Kapusta (41) and others
# Via Dominik Kapusta (9) and others
* main: (138 commits)
  Make sure when we set custom config url, we don't expect etag in return (#1994)
  Add PixelKit source parameter (#1989)
  Fix internal user toggling (#2000)
  Show alert and display warning icon in Sync Settings when data syncing is disabled (#1996)
  DBP: Integrate subscription account authentication to DBP (#1995)
  Improve bookmarks html reader (#1986)
  Add Sync feature flags (#1992)
  Add daily stats pixel (#1993)
  Do not reload DBP tab when switching to it (#1942)
  Fix: external application requests via redirect URLs shows wrong origin. (#1900)
  Update clean-app.sh to work on macOS Sonoma and include NetP containers (#1988)
  Fix: "SwiftLintPlugin" must be enabled before it can be used (#1987)
  Prevent VPN server list persistence failures (#1985)
  add test can remove data (#1980)
  Remove VPN upgrade card (#1983)
  Fix low-res VPN warning asset (#1984)
  DBP: Fix unreliable date tests (#1981)
  Add search retention pixel for NetP (#1964)
  Sabrina/sync e2e tests (#1959)
  swiftlint build plugin (#1318)
  ...

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/Application/AppDelegate.swift
samsymons added a commit that referenced this pull request Dec 22, 2023
# By Dominik Kapusta (2) and others
# Via GitHub
* main:
  Make sure when we set custom config url, we don't expect etag in return (#1994)
  Add PixelKit source parameter (#1989)
  Fix internal user toggling (#2000)
  Show alert and display warning icon in Sync Settings when data syncing is disabled (#1996)
  DBP: Integrate subscription account authentication to DBP (#1995)
  Improve bookmarks html reader (#1986)
  Add Sync feature flags (#1992)
  Add daily stats pixel (#1993)
  Do not reload DBP tab when switching to it (#1942)
  Fix: external application requests via redirect URLs shows wrong origin. (#1900)

# Conflicts:
#	DuckDuckGo/Statistics/PixelEvent.swift
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