Directory ignore pattern not working in upignore [bug]

ask1234560 commented 9 months ago

Dotdrop version (and git commit if run from source): v1.14.0.r8.g9b62d57-1

Describe the bug Ignore not working with directory in upignore

Attaching the video.

https://github.com/deadc0de6/dotdrop/assets/25929911/20206407-2b55-4c0c-b80d-b2a05c0060a7

Expected behavior

/home/ananthu/.config/dotfiles_bspwm/dotfiles/config/mpv/watch_later should be ignored.

Additional information

The relevant part of the config file

config:
  backup: true
  banner: true
  create: true
  dotpath: dotfiles
  keepdot: false
  link_dotfile_default: nolink
  link_on_import: nolink
  longkey: false
dotfiles:
  d_mpv:
    src: config/mpv
    dst: ~/.config/mpv
    chmod: '700'
    cmpignore:
    - '*/watch_later'
    upignore:
    - '*/watch_later'
profiles:
  arch:
    dotfiles:
    - d_mpv

Dotdrop's execution with the debug logs (--verbose)

$ dotdrop update --verbose

Attaching the debug log. debug.log

Thank you @deadc0de6 for dotdrop. If you need any other info please feel free to ask.

deadc0de6 commented 9 months ago

Hello, Thanks for submitting this. Could you try with '*/watch_later/*?

ask1234560 commented 9 months ago

Hi,

I used this -

config:
  backup: true
  banner: true
  create: true
  dotpath: dotfiles
  keepdot: false
  link_dotfile_default: nolink
  link_on_import: nolink
  longkey: false
  d_mpv:
    src: config/mpv
    dst: ~/.config/mpv
    chmod: '700'
    cmpignore:
    - '*/watch_later/*'
    upignore:
    - '*/watch_later/*'
profiles:
  arch:
    dotfiles:
    - d_mpv

Compare - Update -

But the watch_later is not added -

deadc0de6 commented 9 months ago

@ask1234560 I was able to reproduce this and indeed it was only a problem of output as the directory is ignored but we still get an ouput saying it was added.

I'm working on a fix in PR 419.

deadc0de6 commented 9 months ago

The fix has been merged in master. Thanks again for your help on this!

ask1234560 commented 8 months ago

Hi @deadc0de6, thanks for working on the fix but i still face the issue.

dotdrop version - 1.14.0.r32.g4044dab-1

1st case

config:
  backup: true
  banner: true
  create: true
  dotpath: dotfiles
  keepdot: false
  link_dotfile_default: nolink
  link_on_import: nolink
  longkey: false
dotfiles:
  d_mpv:
    src: config/mpv
    dst: ~/.config/mpv
    chmod: '700'
    cmpignore:
    - '*/watch_later/*'
    upignore:
    - '*/watch_later/*'
profiles:
  arch:
    dotfiles:
    - d_mpv

Output -

Result - compare shows incorrect output

2nd case As per the current documentation for ignoring directory


config:
backup: true
banner: true
create: true
dotpath: dotfiles
keepdot: false
link_dotfile_default: nolink
link_on_import: nolink
longkey: false
dotfiles:
d_mpv:
src: config/mpv
dst: ~/.config/mpv
chmod: '700'
cmpignore:
- '*/watch_later'
upignore:
- '*/watch_later'
profiles:
arch:
dotfiles:
- d_mpv



Output -
![image](https://github.com/deadc0de6/dotdrop/assets/25929911/c90b5622-f452-4117-ba0f-3b161009673f)

Result - 
compare works correctly, update breaks and adds the folder `watch_later` with the contents. 

I think we should go with ignore pattern specified in the documentation. I would be happy to provide any other info you need.

deadc0de6 commented 8 months ago

@ask1234560 Thanks for re-submitting this and sorry you're having troubles with dotdrop.

The naive fix is to use for both */watch_later*. It would work in your case but it would backfire if you have for example a directory named watch_later_but_keep_me that you don't want to ignore, so I wouldn't recommend using that.

I would however suggest to use the following as a temporary fix:

*/watch_later for compare: the reason is that compare is done recursively. Going deeper in the tree only if the parent is not filtered out by the pattern. In your case once we reach ..../watch_later, that pattern is matched and the sub-tree is not explored further. On the other hand using */watch_later/* doesn't work since when reaching the ..../watch_later leaf, the pattern is not matched although any subsequent files (within the watch_later directory) would actually match and be ignored.
*/watch_later/* for update: update takes a different approach and will sequentially handle any files individually. You thus need a pattern that will match every paths containing /watch_later/. Therefore using */watch_later won't work since any files within watch_later like for example watch_later/somevid is not matched by the pattern.

I hope this is clear enough. However this gives you the explanation but I agree that it is not optimal and I'm working on trying to find either an unified way of performing those actions with ignore patterns or do a better documentation on how this works. I will get back to you when I have something better.

deadc0de6 commented 8 months ago

I have refactored the way the ignore patterns are applied (as well as the doc) in the refactor ignore pattern PR.

*/somedir/* for directories
*/somefile for files

Now it's consistent and would be applied correctly.

deadc0de6 commented 8 months ago

I have just merged refactor ignore pattern PR which should fix some inconsistencies in handling the ignore patterns across the different actions.

It also now supports both formats for the directories (*/somedir/* and */somdir) although it is recommended to use the proper */somedir/*.

ask1234560 commented 8 months ago

Thanks @deadc0de6 for the fix.

deadc0de6 / dotdrop

Directory ignore pattern not working in upignore [bug] #418