search

alphapapa / org-web-tools

View, capture, and archive Web pages in Org-mode
GNU General Public License v3.0
647 stars 33 forks source link

Fixed for org 9.4 #39

Closed skedonk closed 1 year ago

skedonk commented 4 years ago

Fixed 3 lines in org-web-tools.el to make it work with latest

alphapapa commented 4 years ago

How do these changes affect functionality on earlier Org versions?

skedonk commented 4 years ago

It not backwards compatible. I'll send you one that is in few minutes

bcc32 commented 4 years ago

If this gets merged, I think it obsoletes #37.

skedonk commented 4 years ago

Okay, fixed (not checked though) Thanks for the useful library

alphapapa commented 4 years ago

@bcc32 Thanks for mentioning that.

@skedonk Thanks, but there is a better way to handle this. Rather than duplicating many lines of code and testing the version every time that function is called, the pattern followed in #37 should be followed here: a new defconst whose value is conditional on the current Org version, and is defined at load time.

Probably the best thing to do would be to update #37 for the changes in Org 9.3 and 9.4. I don't want to break compatibility with earlier Org 9.x versions yet.

alphapapa commented 4 years ago

By the way:

Okay, fixed (not checked though)

By "not checked" I guess you mean, "I haven't tested it." Please do not submit untested code in PRs. It only takes a few seconds to test it. If you won't take due diligence to test your patches, then you should generally expect the maintainer to dismiss your patch out-of-hand. It would be better to just report a bug in that case.

davidszp commented 4 years ago

Skedonk's PR worked for me (using org 9.3), with the following modification:

(if (< (string-to-number org-version) 9.3)

instead of the suggested

(if (< (string-to-number org-version) 9.4)

alphapapa commented 1 year ago

I don't know if this PR is still relevant, but the code is somewhat bogus (e.g. setting global variables instead of lexically bound ones), so I'm closing.

Please file a new bug report if one still exists.