Skip to content

Revival of menu restructuring project #3638

Closed
skef wants to merge 3 commits intomasterfrom
menus-2019
Closed

Revival of menu restructuring project #3638
skef wants to merge 3 commits intomasterfrom
menus-2019

Conversation

@skef
Copy link
Copy Markdown
Contributor

@skef skef commented Apr 7, 2019

Summary

This pull request is a proposed substitute for #2724, with branch menus-2019 the substitute for new-menus.

The new branch starts with a squash of the old one, followed by a commit with my updates. Those updates are documented in #3637, which I submitted and self-approved for record-keeping purposes.

@davelab6: We could use your help and expertise in finishing this up.

Process Proposal

(This section might more accurately be called "How to protect @skef's delicate psyche through this review and update process.)

This pull request represents the possibility of merging the new code into the master branch. It's therefore natural to see this as the context for asking the submitter to make a bunch of changes. Please don't do this. I don't "own" it and I don't really want to "defend" the judgment calls I made to get to this point.

Importantly, however, I am not at all saying "all these changes must be accepted as is". My request is instead that we work together on getting this finished using new issues and pull requests, the latter being requests to submit to the menus-2019 branch. So:

  1. If you have an individual concern and know how to address it in code, submit a pull request for branch menus-2019 and the change can be discussed there.
  2. If you don't have code, open an issue to discuss that. Once there is a consensus I'll be happy to help write some code (as long as the problem isn't too huge).

In either case, mark the new request or issue with the menus-2019 label.

That leaves this pull request as a context for general discussion of how ready the branch is to be integrated.

lpsandaruwan and others added 3 commits April 7, 2019 03:13
Add BitmapView to new pattern
Add BitmapView default hotkeys
Update to "new menus" project
@skef skef added the menus-2019 Revival of menu restructuring project label Apr 7, 2019
@jtanx jtanx mentioned this pull request Apr 7, 2019
@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

So, at the code level this is mostly a book-keeping type of project. The exceptions mostly have to do with the *listcheck functions, which have assorted other roles but primarily do the job of disabling inapplicable commands.

I've been thinking about this a lot and it makes my head hurt.

In terms of code organization, it would be ideal if each command had a separate function to call that determines whether it can be run, and (probably) initializes the parameters needed to run it. Maybe some commands should share the function, but only if that made sense at the code level. Then when a menu is activated it could also call that function to determine if the line should be enabled or disabled (Based on a mapping of line to command, which is already present in the code in the form of MID_ defines.)

Unfortunately, that organization style could lead to looping through various structures dozens of times, and if those structures are large enough it might start taking too long to put the menu up. So from a performance perspective you want a single or small number of loops that calculate the conditions all at once.

That's sort of what the old code did, except that different types of checks tended to be grouped together. (It almost seems like what menu some command appeared in was often linked to how to check whether it was enabled ... ) By moving commands to better UI locations, the new design eliminates many of these coincidences.

In most cases I could just move checks around easily. charview.c has most of what expensive looping checks there are, and some of these mostly remained in the same pattern. The mess wound up condensing into cv_pointlistcheck, which is a sort of jumbo-checker used across various lists. It's not pretty, and it should be reviewed carefully. (A better solution would be nice, but that's for the angels.)

Other than that, reviewing is the usual process of actually testing stuff. Someone who knows more about CID and Multiple Master fonts would be a big asset in this area. I have tried the normal bitmap strikes out and it seems to work. The BitmapView, which wasn't even edited before, seems to be in good shape.

@frank-trampe
Copy link
Copy Markdown
Contributor

@skef, the big issue here is not in the technical changes but in the actual changes to menus. Any substantive change will require an easily comprehensible list of changes for review on the user list and a consensus around those.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

@frank-trampe My goal is just to get this started up again roughly where #2724 petered out. The further changes on top of that are modest compared with the state of new-menus.

Or is there a "it's not 2016 anymore" project-management implication I'm missing?

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

Maybe I misunderstand -- is it updated hierarchy description that's needed? It's pretty self-evident just opening the program, but I'm happy to put a summary together or to update the existing google doc spreadsheet.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

As I say in more detail in #3637, in terms of "Customer-facing" changes there are:

  1. The New menu structure itself, and
  2. Hotkey support, with defaults, added to BitmapView

Other than that I'll I did was try to finish the existing design making a few changes when they made sense. The design wasn't quite complete, in the sense that a number of existing menu items had no assigned location, so some differences were inevitable.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

@ctrlcctrlv You're our resident spiro expert and do lots of font development. Want to give this an initial test-run?

@ctrlcctrlv
Copy link
Copy Markdown
Member

Wow, you mentioned me while I was typing a reply :-)

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

Total coincidence!

@ctrlcctrlv
Copy link
Copy Markdown
Member

I can help test this yes. Is the Google Doc in #2724 current? Some of the changes in that seem dubious to me. I'd err on the side of moving as few things around as possible. For those who are serious users of FontForge, given how rarely it changes, we're really use to where things are and might even have muscle memory around certain actions.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

Those documents are current in the sense that there are no newer versions of them (that I'm aware of).

The spirit of this change is definitely "let's restructure everything and provide a better foundation". Not everyone is going to like that, especially the most experienced users. So if you can, keep both categories of feedback in mind but separate the "I don't like this" stuff from the "it doesn't work" stuff. The latter is what I need to look at soonish the former will contribute to the inevitable upcoming food-fight. "Hurt Feelings All Around™!"

I guess I'll say that I picked this up because the new structure is much more comprehensible "by inspection". Anyone fairly familiar with Desktop-style GUI software will be able to pick up FontForge more easily after these changes. I'm also sympathetic to the muscle-memory point, but for experienced users that's primarily a matter of getting the hotkeys the same and only secondarily a matter of menu position.

@davelab6
Copy link
Copy Markdown
Member

davelab6 commented Apr 7, 2019

The spirit of this change is definitely "let's restructure everything and provide a better foundation". Not everyone is going to like that, especially the most experienced users.

I'd err on the side of moving as few things around as possible.

The spirit of this change is definitely "let's restructure everything and provide a better foundation". Not everyone is going to like that, especially the most experienced users.

Right. The current FontForge menu layout is incoherent, and my experience teaching FontForge to 100s of people new to typeface design and font editing via www.craftingtype.com is that most new users are horrified and move on to editors which have coherent layouts.

Those who are already serious users of FontForge with muscle memory around certain actions, are the 'survivors' of running the current UX gauntlet, and it will probably be annoying for them to experience a new version with new menu layouts.

For me, that is not a good reason to err on the side of moving as few things around as possible. Kindly, that is entirely missing the point :)

In the same way that making it really easy to edit the hotkey definitions, by having them all in one file - and making it easy to change the file - I would love to have menu definitions also all in 1 file, and easy to change that file.

Then there can be a fontforge_menus_classic.csv for people who actually want the current menu tree, and a fontforge_menus_2019.csv for people who want a more coherent menu tree.

And then perhaps the latter can be introduced as the secondary option, and the former kept as default, for a few releases, so that users have time to try out the new system, report issues with it, and so on.

@davelab6
Copy link
Copy Markdown
Member

davelab6 commented Apr 7, 2019

I reviewed the commits briefly and currently this PR is hard coding the changes to the menu tree.

It looks like each GMenuItem2 has a tabular format, since there are many lines (rows) with values, like this:

{ { (unichar_t *) N_("_Font Info..."), (GImage *) "elementfontinfo.png", COLOR_DEFAULT, COLOR_DEFAULT, NULL, NULL, 0, 1, 0, 0, 0, 0, 1, 1, 0, 'F' }, H_("Font Info...|No Shortcut"), NULL, NULL, CVMenuFontInfo, MID_FontInfo },

My suggestion remains to rewrite GMenuItem2 so that it can take that info from a spreadsheet, and perhaps have 1 CSV file per GMenuItem2; or perhaps a even use ODS with one tab sheet per GMenuItem2, so the whole menu layout can be in one theme file.

I also guess that the share/default file needs integrating into those sheets; I even guess that GMenuItem2 might have the 2 appended since I guess something similar to what I describe here was done to create that file in the first place. I have no idea, though :)

@davelab6
Copy link
Copy Markdown
Member

davelab6 commented Apr 7, 2019

Finally, I am not looking at the email inbox where my Github notifications are sent very much these days, and it just so happens that on a lazy sunday afternoon I came across this. If I am slow to respond, ping me on twitter or direct email :)

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 7, 2019

My suggestion remains to rewrite GMenuItem2 so that it can take that info from a spreadsheet, and perhaps have 1 CSV file per GMenuItem2; or perhaps a even use ODS with one tab sheet per GMenuItem2, so the whole menu layout can be in one theme file.

@davelab6: unfortunately it's not quite this simple. The menu structure itself can indeed be manipulated this easily, and that's the level the earlier process development had gotten to. But the enable/disable code -- the code that determines whether to "grey out" each entry based on the current data and selection status -- won't automatically come with it. That's where I spent quite a bit of the work on this revival, as noted above and as evident in the code changed. It's also absolutely crucial to usability -- having options that seem applicable but aren't would be endlessly confusing to any user, but particularly those starting out.

It may seem in that case that the best solution would be to make that code entirely modular, so that it could always be "found" in any given menu rearrangement. But unfortunately that modularity would have performance implications. There are possible compromises -- I've thought a bit about this -- but they would require some clever rearchitecting and then reimplementation. I'm afraid this is just one of those "software is complicated" sorts of things. (And one in which FontForge's use of C is actually an advantage -- as the relative speed of looping through structures in C with pointers affords a wider amount of potential solutions within the performance window needed to bring the menu up acceptably quickly.)

@frank-trampe
Copy link
Copy Markdown
Contributor

I don't remember all the issues in which we discussed this before, but this was and remains something that is unlikely to arrive at a sufficiently strong consensus to allow changes. It is important to remember that a lot of our stakeholders are not developers and do not have the capacity to read or to compile code. If we are to do something like this, the proposal needs to be accessible to users, and any final result needs to have strong support among the user base.

The Google Doc is probably close enough to circulate (in non-edit mode) for that purpose. I think that I had a few serious gripes about the proposal. I cannot find them but will rereview once the discussion opens. We have several times looked at the possibility of dynamic menus to allow old and new to co-exist. That is likely the only way to make this sort of thing viable.

I know that it's tempting to look fondly at the days of code churn, but those were not happy times for our users, and we must avoid repeating them whenever possible.

@ctrlcctrlv
Copy link
Copy Markdown
Member

@skef I think I'd only really support this if it came with interactive hotkey editing then. Because I think, to be frank, that you wrong. Much of my work in FontForge is done with the mouse. Not knowing how to set hotkeys properly until recently was why, as well as frustration with the outdated and conflicting hotkey documentation. We literally had to comb through the source code (#3605) recently to try to figure out why certain hotkeys weren't working, and users have asked here before whether custom hotkeys even work at all. (#3066) I've used FontForge since high school (for me seven years ago), by the way, and in that entire time, if it didn't have a default hot key, I'd use the mouse.

The idea that creative people who develop fonts will be able to do this most of the time is honestly absurd. Even George Williams admitted he was much better at programming than typeface design; I myself only do revivals because I'm just not good at it either, despite my love for type.

So yes, if you change around all the menus, a lot of memory among power users is going to be lost.

As @davelab6 wrote, and I agree with him, we should aim to help people better learn to use FontForge, and if the menus are an impediment to that, then I won't stand in the way of change. But to get this over the line we're going to need to make it worthwhile for power users too; after all, we speak loudest.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 8, 2019

@frank-trampe: My understanding is that FontForge is now a contributor-supported open source project. Can you explain the process by which you expect contributors to contribute? Because I read #2724, the related docs, and the other issues relating to these changes, and I have not run across any statement or even intimation of a blocking requirement to modularize the menu system.

Look at #2724. It has been sitting in the queue for three years, 2.5 of those unmodified. Please point to any indication it is blocked on something other than completeness of implementation. A single not-behind-the-scenes comment about anything else that stands in the way of integrating it. 2.5 years!

Here's my guess: You never liked this proposal, you couldn't say "no" to @davelab6 in 2016 because of his project involvement level, now you can. But putting all that in the issue would just be so rude. With luck they'll be no other contributors to FontForge and you'll never have to discuss it!

Fine. But I'm just trying to help, and wasted 4-5 days of effort because of the shitty way you run this project. Put some effort into maintaining your issues and pulls or you're just tending an attractive nuisance.

@ctrlcctrlv
Copy link
Copy Markdown
Member

@skef Even though your reply wasn't to me I hope you don't feel like I'm shitting on your efforts by asking for something (hotkey editor) that might be more difficult to do than making the menu system modular (which I would also support). I should have made that more clear.

I at least really appreciate all you do for the project and all the help you've given me. Even though I might not love this proposal as-is, I appreciate the work you've put into reviving it, and hope it gets merged in some form.

☮️ 🕊️

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 8, 2019

@ctrlcctrlv Adding hotkeys to the themes editor would be nice, but it seems like it would be sufficient to address the problems you raise by 1) fixing the docs and 2) keeping the default hotkeys file up to date and referencing it in the documentation. My intent on making the strings exactly match what's in the menus was also part of that improvement -- with those changes and very basic info (that the string needs to start with e.g. CharView.Menu) you can add a hotkey by just looking at the menu itself.

The other point I'm making, and what I'm angry about, is not at the code level but the project management level. An open source project needs basic management to avoid being abusive to potential contributors. FontForge is currently failing to meet that level. @frank-trampe might say "you should have mailed the dev list asking". I didn't mail the dev list because I've sent messages there before that went unanswered. The issue and pull request lists are supposed to be organized for potential contributions. The issue list here is somewhat of a joke -- over a thousand open issues not prioritized or curated except by occasional whim. There was no indication anywhere in the issue or the supporting documentation of a requirement to modularize the menu subsystem.

Suppose I were to do the extra modularization work. To do that I would need to change a lot more stuff. How the hell could I have confidence that that effort would not also be wasted? How is someone supposed to contribute to this project? What is the model? If the answer is "you do stuff, and once it's ready enough to peak our interest, we'll start discussing our requirements", that's shitty.

@frank-trampe
Copy link
Copy Markdown
Contributor

@skef, there is no hidden agenda here, and it is unproductive to suggest so.

We had a contemporary discussion of the original proposal prior to #2724. I did indeed have some particular objections to the proposed restructure as a user, but I'm sure that we could have addressed those. The bigger issue, and the reason that I did not spend my time (which was much more plentiful then) to take over where others left off, is that I did not see a way to reach a point that would achieve consensus among users on any set of significant changes to the menu system.

I'm as short for time as you are, and I am just as sensitive to the pain of finding some of it to be wasted. I still hope that we can make something of this effort. But we need to take into account the fact that we have a large community of people, maybe 50,000, who use the software, and changes that are not sensitive to their needs can also take a lot of their time. If I did not mention this on #2724, it was because I felt I had already said that too many times.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 8, 2019

I guess that's one way of looking at it.

Another is that you haven't really been doing much FontForge development in recent years, and one of a very small number of people who happened to wander into the project that supports those 50,000 users is wandering back out, because the project isn't being managed in a way that even attempts to avoid wasting people's time.

I do some python improvements. These are the website documents on scripting, which no one has a plan for updating: http://fontforge.github.io/en-US/documentation/scripting/ . So unless a user knows to google "fontforge python", and ignores the message "this is part of the old website" at the top, that work goes into a hole.

Was @lpsandaruwan being paid? If so, whose money was being wasted on work that was never going to meet your requirements? If not, why were you wasting his time? Why was he encouraged to start working again in 2016?

Wel -- alright. It looked to me like this was a project that could use some help, but it seems to be in implicit maintenance mode and run by people too busy to manage it. I'll go back to chewing on Libertinus.

@skef skef closed this Apr 8, 2019
@frank-trampe
Copy link
Copy Markdown
Contributor

@skef, with regard to your comment to @ctrlcctrlv, no user could deeply object to modularizing the menu system since it does not take away the menu structure. I think that there are two major questions. How complicated would it be to implement? And how many users would actually make use of it by giving an alternate menu design a chance?

My plan, when I was considering implementing the modular menu system, was something like this.

  • Create script bindings for all menu item actions and a mechanism to check which of them are enabled/disabled (maybe one dedicated function for all of them or maybe an option for each action handler function).
  • Create a tool that converts XML to a menu that handles names, icons, and actions (by script name), to be run at start-up according to a path in the preference file.
  • Replace any code that updates menu item enabled/disabled status with a call to a central handler.

It seemed rather labor-intensive, but there may be a better way to do it. If there is a feasible way to do it, we can ask on the mailing list how many people would try using a new menu structure.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 8, 2019

No user would object, but you personally have expressed concern in the past about changing function prototypes. A massive change like that is just an invitation for everyone else to say "no, I don't like this changing, no I don't like that changing, No, we don't use designated struct initializers -- someone may have a compiler from 1968."

But never mind, discuss all that with someone else.

@jtanx
Copy link
Copy Markdown
Contributor

jtanx commented Apr 8, 2019

Facing the reality of it, @skef has raised some good points - this project has been neglected, and we haven't exactly been making it easy in the slightest for anyone to contribute. Opposing change for fear of possibly breaking someone's workflow, or angering some, yet where this is a benefit otherwise is just not productive. There are many things I would have liked to have changed, which would have made life easier, yet held back on because of that inertia to change. That's not to say we should just merge everything as-is, but we should not be letting legacies of the past hold us back.

We already have few (to effectively none, nowadays) active contributors, so it's a real crying shame to be at the point of losing yet another.

There's a lot that can be done to manage the project better, but as pointed out, no one of late has actually spent the time to do that. At the same time, we've all got things to do outside of this, and with so few contributors, perhaps that neglect was inevitable, or at least a very difficult hole to get out of.

With regards to this issue in particular, there was definitely some (but perhaps brief) discussion on the concept, which never made its way back into the PR. Doing that (or closing out old issues) perhaps might have made it clearer where it stood at (that, or trawling through the mailing list).

Personally, I have no objection to the change, but what would be non-trivial (given the size) is verifying that all the prior functionality is still there in some form, and that we haven't somehow broken the interaction with it in the process. Whether or not it's worth trying to somehow retain the option of the old layout, or providing something to help the transition - perhaps that might be something to look into.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 8, 2019

In this archive mparse (when run in fontforgeexe) goes through the four files looking for menu entries and builds a perl datastructure for each. (It prints the lines it can't parse, mostly because of commas in quotes and such, so that the data from those could be added manually or, more likely, checked by hand.) manal can then open the datastructures built for the old and new versions, to "ask questions" about the differences. This is how I verified that all the existing function/MID combinations had a new home. m.zip

It would be easy to enhance the script to verify things like: Does every entry in a given menu have a unique underscore value? Is it the same as the last GTextInfo field? Although for the latter you'd have to add that field to what mparse saves by putting parentheses around it and adding it to $t. (I assume that field is irrelevant and that the code has been updated somewhere to extract the letter using the underscore itself, because the existing entries in master are otherwise so broken as to make the system unusable. But I haven't verified that and matched the entries up anyway for documentation purposes. There could be a few typos.)

It would also be easy enough to write a variant of manal to produce a summary of what has moved to where.

It's harder to write a script to verify that the mapping of MID_* to *listcheck functions is proper (because those functions are more accurately parsed than just regex-ed). But since the way those functions work is grouped by menu, verifying isn't hard to do by inspection. (It's harder to get it close to right in the first place.) The hardest thing is to verify that the disabled calculation has been properly preserved, but as I noted earlier the hard case is a thorough review of cv_pointlistcheck. Most of the checks elsewhere are one-liners, perhaps preceded by a short and non-looping calculation block between the declarations and the switch statement.

@frank-trampe
Copy link
Copy Markdown
Contributor

@jtanx, good digging. I knew that there had been a conversation on that. As you mention, there's no great way to run an underfunded and understaffed project, but there are a lot of little things that we can do to improve.

@frank-trampe
Copy link
Copy Markdown
Contributor

@skef, that was a fast turn on the menu verifier! I can't test it just now, but good work.

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 8, 2019

I didn't write those just now; they're the unmodified scripts I used in the latter part of my implementation.

Whatever the concerns about the general approach, while there are probably a few bugs the menus-2019 branch is in good shape. It meets the quality level of my other pull requests over the past few months, which I'm not an unbiased judge of but I consider to have been of good quality.

One thing I did not verify -- because I didn't have one handy -- is whether the python registerMenuItem enable_function is still wired up correctly. The way the that code is written the changes I've made shouldn't affect it, but it's always possible something strange changed or that it didn't work in the first place.

@ctrlcctrlv
Copy link
Copy Markdown
Member

Why was this closed? Should I still test this branch?

@skef
Copy link
Copy Markdown
Contributor Author

skef commented Apr 9, 2019

It seems more likely than not that testing this particular code for correctness is a waste of time, so I would say "no" -- you have more productive work of your own to do, in your font development and your other improvements to FontForge.

@ctrlcctrlv
Copy link
Copy Markdown
Member

@skef Speaking of that, would you be open to some kind of direct communication where we could discuss the plugins idea and I could possibly get help from you? I'm working on a monster commit right now to replace #3617, master...ctrlcctrlv:glyphinfoED, and could have used your expertise several times but didn't want to open an issue.

If you're open to me sending you personal emails, just email copypaste@kittens.ph so I know (a) you're open to it and (b) your address

Thanks either way

@skef skef mentioned this pull request Apr 9, 2019
4 tasks
@skef skef mentioned this pull request Jul 26, 2019
@davelab6
Copy link
Copy Markdown
Member

davelab6 commented Feb 6, 2020

@davelab6: We could use your help and expertise in finishing this up.

Given this seems to have been closed out without being merged, I guess the help I offered back in April last year was insufficient at best 😂 Still I'd like to see about getting this sorted, one way or another.

The governance issues seem worth resolving...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

menus-2019 Revival of menu restructuring project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants