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

internal: use wb_comment() in wb_add_thread + deprecations guide #766

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented Aug 24, 2023

THis aims to help users deal better with deprecations

  • Change internal to wb_comment().
  • This will ensure that options(openxlsx2.soon_deprecated) works as intended and does not detect internal function use.
  • An added benefit of Export wb_comment() #758 is also that comment are similar. (both accept text, although fmt_txt() is not supported in threads

just so you know, there will be some internal rework necessary to make comments "visible" but I would wait for a user request for that. #765 (comment)

I squeezed in an enhanced note on the deprecations that we can direct users to should problems arise.

No visual change before and after

Shows when hovering
image

Hidden when cursor is far

image
Edit: the openxlsx2_options name change is just that it was bugging me when navigating in files.. Saw only a single use in the pkg.

Edit2 : wb_add_thread doesn't error when not providing person_id. But it creates corrupt output (can't wb$open() after) in case you want to add protection for that.

@olivroy olivroy changed the title use wb_comment() in wb_add_thread internal: use wb_comment() in wb_add_thread + deprecations guide Aug 24, 2023
@JanMarvin JanMarvin merged commit 2d48564 into JanMarvin:main Aug 24, 2023
9 checks passed
@JanMarvin
Copy link
Owner

Thank you so much <3

@JanMarvin
Copy link
Owner

Edit2 : wb_add_thread doesn't error when not providing person_id. But it creates corrupt output (can't wb$open() after) in case you want to add protection for that.

I wanted to capture this case here. If that's not working, we might have to revisit this. But it is again midnight in Germany and it's been a long week.

openxlsx2/R/class-workbook.R

Lines 3820 to 3824 in 7b65f82

if (missing(person_id)) {
person_id <- getOption("openxlsx2.thread_id")
if (is.null(person_id)) stop("no person id found")
}

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