-
Notifications
You must be signed in to change notification settings - Fork 12
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
Better docs for multi-file mode? #48
Comments
Update: this problem was on my end and is now fixed. So I think the main issues are documenting the relative path thing and fixing deletion of directories that are parents of the "main" source file. Ideally we'd have a mode where there is no main source file, but with proper docs hopefully that's not too big a deal. |
Wow. This was a trip down memory lane :) In summary: I think you are right, but I also think the largest jsdelta client (TAJS,
I think you are right that the documentation is lacking. For this, I'll refer to how TAJS uses jsdelta through a Java wrapper: if (dir != null) {
cmd.add("--dir");
cmd.add(dir.toAbsolutePath().toString());
cmd.add(dir.relativize(target).toString());
} else {
cmd.add(target.toAbsolutePath().toString());
} I interpret this to mean that the
Agreed. I suspect this is an artifact of TAJS being the initial client for the multi file minimization, and that the Some documentation evidence for the TAJS predicates that all expect a (main) file as input: /**
* Interface to be used in {@link DeltaMinimizer#reduce(Path, Class)}.
* <p>
* Should return true iff the file satisfies some predicate. With concrete example predicates.
Again I think this is a TAJS client artifact: I think TAJS creates a fresh directory structure before each jsdelta use, perhaps in a directory like |
Thanks a lot @esbena!! That is really helpful. I think realistically speaking, I won't have time to implement a proper no-main-file directory reduction mode anytime soon. I think better documentation can mostly solve the relative path issue. I think deleting parent directories of the main file is just wrong, but I wouldn't imagine that disabling that should affect TAJS; it just might make the reduction slightly slower. So I may look into that at some point. In the end, I got multi-file reduction working for my case, and it's been really helpful! So I'm very glad this functionality was contributed back. |
Improve path documentation (close #48)
I've been trying to use jsdelta in a multi-file scenario but not having much success. Here's the command I am using:
The first thing that tripped me up is that I think the path to the "main" source file must be relative to
program_dir
, not an absolute path. If this is correct, we should document it.Then, I ran into an issue with
delta_multi.js
trying to delete entire directories, as it would also delete parent directories ofmain.js
, leading to a failure to stat the file. This I think we can fix.My main problem is the following. Really, my analysis is running over all scripts in
program_dir
; there is no "main" source file to be distinguished from the others. When jsdelta is minimizing other files, I still see printed in the output:Is this expected? Or is the search getting messed up somehow because the "Original file doesn't satisfy predicate"?
I think in the multi-file mode, there should be no need to specify a "main" file, and things should just work. Maybe my mental model for how the tool is working is wrong, though.
@esbena any thoughts / memories around this? 🙂
The text was updated successfully, but these errors were encountered: