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

Wish: session-specific LilyPond include paths list #470

Closed
dliessi opened this issue Sep 9, 2014 · 45 comments
Closed

Wish: session-specific LilyPond include paths list #470

dliessi opened this issue Sep 9, 2014 · 45 comments
Assignees
Labels

Comments

@dliessi
Copy link
Collaborator

dliessi commented Sep 9, 2014

Different projects often need different LilyPond include paths.
To avoid manually changing the include paths list every time I switch project, I'd like to have session-specific include paths lists (in the session management window).

There should also be a toggle in the session management window to control whether the general include paths list should be used for the session.
It should be enabled by default for new sessions.
(Of course when in "No Session" the behaviour could not be configurable, so the general list would always be enabled.)

If the general list is enabled for a session, the paths in it should be appended after the session-specific list.

@uliska
Copy link
Collaborator

uliska commented Sep 9, 2014

+1

@PeterBjuhr
Copy link
Collaborator

Good idea! You could assign me for this task if you like...

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 9, 2014

You could assign me for this task if you like...

If "you" means me, feel free to work on this: I don't have much time right now to work on it.
If "you" means @wbsoft, let's wait for his opinion.

@PeterBjuhr
Copy link
Collaborator

@dliessi can you as the OP make the assign? I didn't know that...

Actually I sought for a wording that didn't include 'you' but couldn't come up with one... :)

@uliska
Copy link
Collaborator

uliska commented Sep 9, 2014

No, only project members (i.e @wbsoft) can assign issues.

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 9, 2014

Sorry, I wrongly interpreted your words as "You (Davide) can let me do this if you don't want to do it personally" instead of "You (Wilbert) can formally assign this issue to me if you like".
Until now I hadn't noticed that GitHub issues can be assigned (although it is probably one of the most basic features of an issue tracker); of course, not being a project member I cannot assign issues.

@PeterBjuhr
Copy link
Collaborator

No problem!

We (or rather @wbsoft) haven't used the assign functionality that much. So it was just an idea...

@wbsoft
Copy link
Collaborator

wbsoft commented Sep 10, 2014

I could make you project members.
update: done :-)

@dliessi dliessi added the wish label Sep 10, 2014
@PeterBjuhr PeterBjuhr self-assigned this Sep 10, 2014
@PeterBjuhr
Copy link
Collaborator

Great, thanks!

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 10, 2014

Yes, it's great! Thanks!

@PeterBjuhr
Copy link
Collaborator

If the general list is enabled for a session, the paths in it should be appended after the session-specific list.

Is the order relevant here? When we add paths in the general preferences they are added at the bottom of the list.

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 10, 2014

I believe it is, or at least it should be: from LilyPond's "Usage" manual:

-I, --include=directory
Add directory to the search path for input files.

Multiple -I options may be given. The search will start in the first defined directory, and if the file to be included is not found the search will continue in subsequent directories.

@uliska
Copy link
Collaborator

uliska commented Sep 10, 2014

On 10. September 2014 19:24:37 MESZ, Davide Liessi notifications@github.com wrote:

I believe it is, or at least it should be: from LilyPond's "Usage"
manual
:

-I, --include=directory
Add directory to the search path for input files.

Multiple -I options may be given. The search will start in the first
defined directory, and if the file to be included is not found the
search will continue in subsequent directories.


Reply to this email directly or view it on GitHub:
#470 (comment)

It may matter in the case of files with identical names.

@PeterBjuhr
Copy link
Collaborator

I will create a listbox like the one in the LilyPond preferences for now, and we can discuss this more...

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 10, 2014
…p in dialog

For a new session the paths from the general preferences are added by default.
This can then be edited by the user.
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 10, 2014
Including the session specific include paths to the jobinfo.
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 10, 2014
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 10, 2014
@PeterBjuhr
Copy link
Collaborator

That's a start anyway. Please check it out...

I didn't figure out a way to resize the dialog back when the include paths are rehidden. Maybe someone with better knowledge of Qt can help out!?

@wbsoft
Copy link
Collaborator

wbsoft commented Sep 11, 2014

I should always show the widget but disable it (setEnabled(False)). I think user interface elements should not "vanish", but I did not yet check it out...

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 11, 2014
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 11, 2014
@PeterBjuhr
Copy link
Collaborator

I've now used setEnabled. But I think one alternative would be to have it hidden from the start and when (if) the user unchecks the checkbox then it will be disabled. What do you think?

@wbsoft
Copy link
Collaborator

wbsoft commented Sep 11, 2014

I would always keep UI elements visible, then the UI is more predictable.
You could "indent" somehow a group of options that depends on a checkbox. Or use a QGroupBox that is set checkable.

@PeterBjuhr
Copy link
Collaborator

I think QGroupboxcould be a nice option!

@wbsoft
Copy link
Collaborator

wbsoft commented Sep 11, 2014

I use it at many places, e.g. in the Scorewiz, tab score settings the Instrument Names group. It disables all the contained widgets when its checkbox is unchecked. This is the default behaviour of a checkable QGroupBox.

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 13, 2014
@PeterBjuhr
Copy link
Collaborator

I've now changed to QGroupBox. Not much difference for the user. But a cleaner implementation. And it's perhaps also better in line with the overall graphical experience.

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 15, 2014

When he wants to have session (i.e. project) specific includes they will very often (if not regularly) be in addition to the "default library".

This is generally true, but there are use cases, e.g. collaborative projects (@uliska do you know Das trunkne Lied? :) ), where only some libraries are allowed.
That is actually the use case I had in mind when I requested this addition.

Maybe the solution outlined by @wbsoft is the most simple and flexible: the user can choose (with a checkbox) to append or not the general path list after the session list; if the user needs some of the general paths in a different order, he/she can add them to the session-specific list and drag them to the desired position (I think that there is no need to remove duplicates, from the viewpoint of the LilyPond program).

UI suggestion: if general paths are appended (i.e. the checkbox is checked) then the session list could show also the general paths after the session paths, but grey/uneditable, so it is clear for the user what is going on.

@PeterBjuhr

Would it be better if the button always added the global paths to the present list? Or is the alternative to add another button?

What I meant was to leave the "Revert" button as it is, and I was wondering whether to add another button for the global path copy or not.

With the checkbox idea, the "Revert" and the "Copy" button and the initial copy of the general list aren't actually needed.
However, I think that the initial copy should be kept in place, because it can be useful if someone wants (some of) the general paths sorted in a different way.
E.g. in my use case I already have my personal library and openLilyLib in the general list and for the mentioned collaborative project I need the project-specific library and openLilyLib, but of course I don't want to risk using something from my personal library.

The "Copy" button is not worth adding, in my opinion: it would be needed only if one deletes the initial list entries, adds some other paths and then wants to add the general paths again; I think it is harder to explain (in a short and meaningful name) what the button does, than to manually add the missing paths.

I'm still uncertain whether the "Revert" button should be kept or not.

@PeterBjuhr
Copy link
Collaborator

This is generally true, but there are use cases, e.g. collaborative projects (@uliska do you know Das trunkne Lied? :) ), where only some libraries are allowed.
That is actually the use case I had in mind when I requested this addition.

Yes, I thought it had something to do with that... :)

@uliska
Copy link
Collaborator

uliska commented Sep 15, 2014

Am 15.09.2014 11:48, schrieb Davide Liessi:

This is generally true, but there are use cases, e.g. collaborative
projects (@uliska https://github.com/uliska do you know Das trunkne
Lied? :) ), where only some libraries are allowed.
In what way are only some libraries allowed there?

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 15, 2014

In what way are only some libraries allowed there?

The following sentence, that I somehow placed in the wrong point of the comment, explains what I meant:

E.g. in my use case I already have my personal library and openLilyLib in the general list and for the mentioned collaborative project I need the project-specific library and openLilyLib, but of course I don't want to risk using something from my personal library.

(If I understand correctly, for "Das trunkne Lied" we only use openLilyLib and the project-specific library: if we need other functions they must be included either in openLilyLib or in the project library.)

I'm not saying that this kind of mistake (using something from the personal library) is likely to happen, but I prefer that LilyPond gives a "File not found" warning/error, may it happen.

Another use case is that one may need different versions of the same library (thinking of openLilyLib here) for different projects.
Again, I'm not saying that this is likely to happen, but it may happen.

@wbsoft
Copy link
Collaborator

wbsoft commented Sep 16, 2014

I think a checkbox whether to also add the global includepath could be enough. The session-specific include paths are specified first anyway, so that LilyPond will prefer included files in that directories. Showing them in a greyed-out way could be done, but might be overly complicated.

@PeterBjuhr
Copy link
Collaborator

I've already implemented that suggestion. I'll return shortly with a pull request.

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 16, 2014
…ce checkbox

Give user option to replace or add to global paths.
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 16, 2014
@PeterBjuhr
Copy link
Collaborator

I like that you as a user can see what paths are included from the global settings (greyed-out). That gives a better overview. But it can perhaps be confusing when you shift over to the 'replace mode'. Then the global paths are still there but (as in previous implementation) actively replacing the global settings (not greyed-out).

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 16, 2014

Then the global paths are still there but (as in previous implementation) actively replacing the global settings (not greyed-out).

I tried the changes in pull request #480 and indeed activating and deactivating the "replace mode" actually changes the session-specific list.
I do agree that this is confusing, and I changed my mind regarding the initial copy of the general list when using "replace mode": I think that the checkbox should only show or hide the greyed-out paths without changing the list (i.e. no "hard copy" of the general list).

Apart from this detail, I like this new feature and the session import/export #460 very much.

@PeterBjuhr
Copy link
Collaborator

Apart from this detail, I like this new feature and the session import/export #460 very much.

Great, thanks!

I think that the checkbox should only show or hide the greyed-out paths without changing the list (i.e. no "hard copy" of the general list).

Ok, then if the user wants to edit the global paths for a session (like in your use-case) he or she has to push the revert button to get the global paths. I hope this workflow will be clear!??

One other thing I thought about: when you are in 'add mode' (i e replace not checked) the clear button shouldn't delete the greyed-out global paths because they're of course still active.

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 17, 2014
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 17, 2014
@dliessi
Copy link
Collaborator Author

dliessi commented Sep 17, 2014

Ok, then if the user wants to edit the global paths for a session (like in your use-case) he or she has to push the revert button to get the global paths. I hope this workflow will be clear!??

Well, automatically copying the global paths in some situations turned out to be very confusing, in my opinion.

I already was convinced that the "Copy" button was not worth adding and was uncertain about the "Revert" button: if we get rid of the automatic global path copy, then I think that also the "Revert" button is unnecessary.
Also, in my opinion, either the user expects that "Revert" and "Clean" do the same thing (until he/she tries "Revert") or the user expects "Revert" to undo the last change(s) (but there is the "Cancel" button for that).

We could consider adding a "Copy global paths" button, but I'm still convinced that for the user it would be harder to understand what it does (without reading the user guide) than to manually add the paths.

P.S. Maybe you could rename "Clean" to "Clear": it seems more appropriate to me (but English is not my first language, so I may be wrong).

@PeterBjuhr
Copy link
Collaborator

Agreed, 'clear' seems to be a better name (I also used that name above...)!

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 17, 2014
PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 17, 2014
@PeterBjuhr
Copy link
Collaborator

I renamed the revert button "edit global paths" instead of your suggestion "copy global paths". Because that is basically the functionality we like to have; to be able to edit the global paths for the specific session.

Also now it doesn't remove the existing paths like with the previous revert idea.

But it may still be confusing!? What do you think?

@dliessi
Copy link
Collaborator Author

dliessi commented Sep 17, 2014

I like the change in functionality, and I think that we can keep this button, after all.

I'm just wondering if the label "Edit global paths" conveys the right meaning here: even though it is clear that I'm doing something in the session management window, I expect that the effects of "Edit global paths" are to modify the global paths themselves and not a session-local copy of them.
But maybe this is just me.
Anyone else has opinions (especially @wbsoft and @uliska)?

@PeterBjuhr
Copy link
Collaborator

Agreed, it is still not clear! What about "Get global paths"?

Did you catch that the button is disabled initially and also after the paths are added (there's no point in adding them twice)? Is this good, and should the button be enabled again after pushing 'clear'? Should even the state of the button be remembered till later visits?

@PeterBjuhr
Copy link
Collaborator

That bug fix reminded me that there still are some loose ends here. I try to wrap this up as soon as possible.

@wbsoft
Copy link
Collaborator

wbsoft commented Sep 28, 2014

"Copy global path"?
Note: I prefer "path" over "paths". A search path is already a list of directories.

PeterBjuhr added a commit to PeterBjuhr/frescobaldi that referenced this issue Sep 28, 2014
wbsoft added a commit that referenced this issue Sep 28, 2014
Trying to wrap up and close issue #470
@dliessi
Copy link
Collaborator Author

dliessi commented Sep 30, 2014

@PeterBjuhr I'm sorry I didn't answer in the last two weeks.

I think that this is now very nice and usable.
I would change only one detail:

Did you catch that the button is disabled initially and also after the paths are added (there's no point in adding them twice)? Is this good, and should the button be enabled again after pushing 'clear'? Should even the state of the button be remembered till later visits?

I think that the "Copy global path" button should always be enabled when "Replace global path" is enabled, i.e. it should be enabled or disabled only according to the state of the "Replace..." checkbox.

E.g. if the user copies the global path, adds some other entries, then deletes one of the copied-from-global entries and then wants to copy the global path again to recover the deleted entry, the user needs to close and open the session manager or disable and enable "Replace global path", because the "Copy global path" button is disabled.
OK, admittedly not a very likely use case. :)

@PeterBjuhr
Copy link
Collaborator

I think that the "Copy global path" button should always be enabled when "Replace global path" is enabled, i.e. it should be enabled or disabled only according to the state of the "Replace..." checkbox.

Yes, that is perhaps the cleanest solution!

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

4 participants