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

#+FILETAGS are ignored by tags-inherited #55

Closed
mskorzhinskiy opened this issue Nov 14, 2019 · 8 comments
Closed

#+FILETAGS are ignored by tags-inherited #55

mskorzhinskiy opened this issue Nov 14, 2019 · 8 comments
Assignees
Labels
Milestone

Comments

@mskorzhinskiy
Copy link
Contributor

I've tried several styles to specify filetags, unfortunately none of them are working. Example file:

#+FILETAGS: :Peter:Boss:Secret:
#+FILETAGS: :Boss:
#+FILETAGS: Secret

* Meeting with the French group      :work:
** Summary by Frank                  :boss:notes:
*** TODO Prepare slides for him      :action:

Inheritance from headline works OK:

(org-ql-search "~/testfile.org"
  '(tags "work")) => 3 items

And none of the filetags are matched:

(org-ql-search "~/testfile.org"
  '(tags "Peter")) => 0 items
(org-ql-search "~/testfile.org"
  '(tags "Boss")) => 0 items
(org-ql-search "~/testfile.org"
  '(tags "Secret")) => 0 items
@alphapapa alphapapa self-assigned this Nov 14, 2019
@alphapapa
Copy link
Owner

Thanks for reporting this. When I wrote the new tags code, I neglected to consider file tags (I hardly use them myself). It shouldn't take too much work to support them.

@mskorzhinskiy
Copy link
Contributor Author

Honestly I am using them very rarely too, I have only one file with only one file tag, so it took some time before I noticed something strange.

I think I just need to find a proper regexp string for filetags and add them to tags-inhereted, and that is all. There is also caching system, I am not sure is there are anything I should about it?

Anyway, if you don't mind, I can prepare a patch for this. It is a nice opportunity for me to learn a bit more about emacs lisp. :-) But I can't promise any ETA.

@alphapapa
Copy link
Owner

I would start with this:

org-file-tags is a variable defined in ‘org.el’.
Its value is nil
Local in buffer cpb.org; global value is the same.

Documentation:
List of tags that can be inherited by all entries in the file.
The tags will be inherited if the variable ‘org-use-tag-inheritance’
says they should be.
This variable is populated from #+FILETAGS lines.

mskorzhinskiy pushed a commit to mskorzhinskiy/org-ql that referenced this issue Nov 17, 2019
mskorzhinskiy added a commit to mskorzhinskiy/org-ql that referenced this issue Nov 17, 2019
mskorzhinskiy added a commit to mskorzhinskiy/org-ql that referenced this issue Dec 12, 2019
mskorzhinskiy added a commit to mskorzhinskiy/org-ql that referenced this issue Dec 12, 2019
mskorzhinskiy added a commit to mskorzhinskiy/org-ql that referenced this issue Dec 27, 2019
@alphapapa alphapapa added this to the 0.5 milestone Jan 3, 2020
@alphapapa alphapapa added the bug label Jan 3, 2020
mskorzhinskiy added a commit to mskorzhinskiy/org-ql that referenced this issue Jan 3, 2020
mskorzhinskiy added a commit to mskorzhinskiy/org-ql that referenced this issue Jan 3, 2020
@alphapapa alphapapa modified the milestones: 0.5, 0.4 Jan 3, 2020
alphapapa added a commit that referenced this issue Jan 8, 2020
Fixes #55.  Closes #57.

Thanks to Mikhail Skorzhinskiy (@mskorzhinskiy).
@alphapapa
Copy link
Owner

@mskorzhinskiy Please test this commit and ensure it fixes the problem for you: 1330fff. The tests seem to indicate that it does, but I'd like confirmation from you to be sure. When you confirm, I'll merge to master.

As you can see, I used a different approach in the code: simply, when at a top-level heading, return org-file-tags as the inherited tags, which then cascade down to child headings.

Most of the changes in the branch are to the test infrastructure to allow testing of file tags without invalidating existing tests or including file tags in their results.

Thanks for your help with this issue.

@mskorzhinskiy
Copy link
Contributor Author

I've tried to run it on my data set and I didn't see any regressions.
Although, I didn't run these tests as I did have some troubles with running them. Will figure them out next time I'll show up with a patch. Sorry about that.
Thank you for your time and passion working on this package.

@alphapapa
Copy link
Owner

Don't worry about running the test suite. I'd just like to confirm that file tags are correctly inherited for you now. Can you test that manually?

@mskorzhinskiy
Copy link
Contributor Author

mskorzhinskiy commented Jan 8, 2020

Can you test that manually?

Eh, sorry. What I really meant by saying I didn't see any regressions is that I just tried to run it manually on my data set and with your patch everything works as I would expect.
The change was tested and I think ready to be merged.

@alphapapa
Copy link
Owner

Great, thank you.

alphapapa added a commit that referenced this issue Jan 8, 2020
Fixes #55.  Closes #57.

Thanks to Mikhail Skorzhinskiy (@mskorzhinskiy).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants